fixup! v8: add v8.startupSnapshot utils

nodejs · joyeecheung · Apr 18, 2022 · Jun 8, 2022 · Jun 13, 2022 · Jun 8, 2022
commit 8b68343854f3c0b26ea94d69c0f34c4ad0feb15b
diff --git a/doc/api/v8.md b/doc/api/v8.md
@@ -882,16 +882,18 @@ occur synchronously in the case of `Promise.resolve()` or `Promise.reject()`.
 added: REPLACEME
 -->
 
+> Stability: 1 - Experimental
+
 The `v8.startupSnapshot` interface can be used to add serialization and
 deserialization hooks for custom startup snapshots. Currently the startup
 snapshots can only be built into the Node.js binary from source.
 
 ```console
-cd /path/to/node
-./configure --node-snapshot-main=entry.js
-make node
- # This binary contains the result of the execution of entry.js
-out/Release/node
+$ cd /path/to/node
+$ ./configure --node-snapshot-main=entry.js
+$ make node
+# This binary contains the result of the execution of entry.js
+$ out/Release/node
 ```
 
 In the example above, `entry.js` can use methods from the `v8.startupSnapshot`
@@ -935,18 +937,22 @@ The resulted binary will simply print the data deserialized from the snapshot
 during start up:
 
 ```console
-out/Release/node
+$ out/Release/node
 # Prints content of ./test/fixtures/x1024.txt
 ```
 
+Currently the API is only available to a Node.js instance launched from the
+default snapshot, that is, the application deserialized from a user-land
+snapshot cannot use these APIs again.
+
 ### `v8.startupSnapshot.addSerializeCallback(callback[, data])`
 
 <!-- YAML
 added: REPLACEME
 -->
 
 * `callback` {Function} Callback to be invoked before serialization.
-* `data` {any} Optional data that will be pass to the `callback` when it
+* `data` {any} Optional data that will be passed to the `callback` when it
   gets called.
 
 Add a callback that will be called when the Node.js instance is about to
@@ -962,7 +968,7 @@ added: REPLACEME
 
 * `callback` {Function} Callback to be invoked after the snapshot is
   deserialized.
-* `data` {any} Optional data that will be pass to the `callback` when it
+* `data` {any} Optional data that will be passed to the `callback` when it
   gets called.
 
 Add a callback that will be called when the Node.js instance is deserialized
@@ -979,7 +985,7 @@ added: REPLACEME
 
 * `callback` {Function} Callback to be invoked as the entry point after the
   snapshot is deserialized.
-* `data` {any} Optional data that will be pass to the `callback` when it
+* `data` {any} Optional data that will be passed to the `callback` when it
   gets called.
 
 This sets the entry point of the Node.js application when it is deserialized

diff --git a/src/api/embed_helpers.cc b/src/api/embed_helpers.cc
@@ -3,6 +3,7 @@
 #include "debug_utils-inl.h"
 
 using v8::Context;
+using v8::Function;
 using v8::Global;
 using v8::HandleScope;
 using v8::Isolate;
@@ -44,8 +45,11 @@ Maybe<int> SpinEventLoop(Environment* env) {
       if (EmitProcessBeforeExit(env).IsNothing())
         break;
 
-      if (env->RunSnapshotSerializeCallback().IsEmpty()) {
-        break;
+      {
+        HandleScope handle_scope(isolate);
+        if (env->RunSnapshotSerializeCallback().IsEmpty()) {
+          break;
+        }
       }
 
       // Emit `beforeExit` if the loop became alive either after emitting
@@ -58,6 +62,11 @@ Maybe<int> SpinEventLoop(Environment* env) {
   if (env->is_stopping()) return Nothing<int>();
 
   env->set_trace_sync_io(false);
+  // Clear the serialize callback even though the JS-land queue should
+  // be empty this point so that the deserialized instance won't
+  // attempt to call into JS again.
+  env->set_snapshot_serialize_callback(Local<Function>());
+
   env->PrintInfoForSnapshotIfDebug();
   env->VerifyNoStrongBaseObjects();
   return EmitProcessExit(env);

diff --git a/src/env.cc b/src/env.cc
@@ -34,6 +34,7 @@ using v8::Array;
 using v8::Boolean;
 using v8::Context;
 using v8::EmbedderGraph;
+using v8::EscapableHandleScope;
 using v8::Function;
 using v8::FunctionTemplate;
 using v8::HandleScope;
@@ -672,33 +673,23 @@ void Environment::PrintSyncTrace() const {
 }
 
 MaybeLocal<Value> Environment::RunSnapshotSerializeCallback() const {
+  EscapableHandleScope handle_scope(isolate());
   if (!snapshot_serialize_callback().IsEmpty()) {
-    HandleScope handle_scope(isolate());
-    Context::Scope context_scope(context());
-    return snapshot_serialize_callback()->Call(
-        context(), v8::Undefined(isolate()), 0, nullptr);
-  }
-  return Undefined(isolate());
-}
-
-MaybeLocal<Value> Environment::RunSnapshotDeserializeCallback() const {
-  if (!snapshot_serialize_callback().IsEmpty()) {
-    HandleScope handle_scope(isolate());
     Context::Scope context_scope(context());
-    return snapshot_serialize_callback()->Call(
-        context(), v8::Undefined(isolate()), 0, nullptr);
+    return handle_scope.EscapeMaybe(snapshot_serialize_callback()->Call(
+        context(), v8::Undefined(isolate()), 0, nullptr));
   }
-  return Undefined(isolate());
+  return handle_scope.Escape(Undefined(isolate()));
 }
 
 MaybeLocal<Value> Environment::RunSnapshotDeserializeMain() const {
+  EscapableHandleScope handle_scope(isolate());
   if (!snapshot_deserialize_main().IsEmpty()) {
-    HandleScope handle_scope(isolate());
     Context::Scope context_scope(context());
-    return snapshot_deserialize_main()->Call(
-        context(), v8::Undefined(isolate()), 0, nullptr);
+    return handle_scope.EscapeMaybe(snapshot_deserialize_main()->Call(
+        context(), v8::Undefined(isolate()), 0, nullptr));
   }
-  return Undefined(isolate());
+  return handle_scope.Escape(Undefined(isolate()));
 }
 
 void Environment::RunCleanup() {

diff --git a/test/fixtures/snapshot/v8-startup-snapshot-api.js b/test/fixtures/snapshot/v8-startup-snapshot-api.js
@@ -15,10 +15,12 @@ const storage = {};
 
 if (isBuildingSnapshot()) {
   addSerializeCallback(({ filePath }) => {
+    console.error('serializing', filePath);
     storage[filePath] = zlib.gzipSync(fs.readFileSync(filePath));
   }, { filePath });
 
   addDeserializeCallback(({ filePath }) => {
+    console.error('deserializing', filePath);
     storage[filePath] = zlib.gunzipSync(storage[filePath]);
   }, { filePath });