From 4642ec55a00f0586a64b0be117ace8750805a589 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Mon, 30 Dec 2024 01:00:56 +0000 Subject: [PATCH 1/2] src: drain platform tasks before creating startup snapshot Drain the loop and platform tasks before creating a snapshot. This is necessary to ensure that the no roots are held by the the platform tasks, which may reference objects associated with a context. For example, a WeakRef may schedule an per-isolate platform task as a GC root, and referencing an object in a context, causing an assertion in the snapshot creator. --- src/node_snapshotable.cc | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index fe04a8ee8d708b..f3b0b8c172b49d 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -973,25 +973,33 @@ ExitCode BuildSnapshotWithoutCodeCache( } }); + Context::Scope context_scope(setup->context()); + Environment* env = setup->env(); + // Run the custom main script for fully customized snapshots. if (snapshot_type == SnapshotMetadata::Type::kFullyCustomized) { - Context::Scope context_scope(setup->context()); - Environment* env = setup->env(); #if HAVE_INSPECTOR env->InitializeInspector({}); #endif if (LoadEnvironment(env, builder_script_content.value()).IsEmpty()) { return ExitCode::kGenericUserError; } + } - // FIXME(joyeecheung): right now running the loop in the snapshot - // builder might introduce inconsistencies in JS land that need to - // be synchronized again after snapshot restoration. - ExitCode exit_code = - SpinEventLoopInternal(env).FromMaybe(ExitCode::kGenericUserError); - if (exit_code != ExitCode::kNoFailure) { - return exit_code; - } + // Drain the loop and platform tasks before creating a snapshot. This is + // necessary to ensure that the no roots are held by the the platform + // tasks, which may reference objects associated with a context. For + // example, a WeakRef may schedule an per-isolate platform task as a GC + // root, and referencing an object in a context, causing an assertion in + // the snapshot creator. + + // FIXME(joyeecheung): right now running the loop in the snapshot + // builder might introduce inconsistencies in JS land that need to + // be synchronized again after snapshot restoration. + ExitCode exit_code = + SpinEventLoopInternal(env).FromMaybe(ExitCode::kGenericUserError); + if (exit_code != ExitCode::kNoFailure) { + return exit_code; } } From c1a5d208e8e9b327d42c804ec5b34cf4884edeaa Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Mon, 6 Jan 2025 23:47:00 +0000 Subject: [PATCH 2/2] fixup! src: drain platform tasks before creating startup snapshot --- src/node_snapshotable.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index f3b0b8c172b49d..fe3fcc7184205f 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -992,10 +992,6 @@ ExitCode BuildSnapshotWithoutCodeCache( // example, a WeakRef may schedule an per-isolate platform task as a GC // root, and referencing an object in a context, causing an assertion in // the snapshot creator. - - // FIXME(joyeecheung): right now running the loop in the snapshot - // builder might introduce inconsistencies in JS land that need to - // be synchronized again after snapshot restoration. ExitCode exit_code = SpinEventLoopInternal(env).FromMaybe(ExitCode::kGenericUserError); if (exit_code != ExitCode::kNoFailure) {