fix(core): Register external references for imports to the SnapshotCreator (#15621)

Several functions used for handling of dynamic imports and "import.meta"
object were not registered as external references and caused V8 to crash
during snapshotting. These functions are now registered as external refs
and aborts are no longer happening.

core/bindings.rs

@@ -19,9 +19,20 @@ pub fn external_references(
   ops: &[OpCtx],
   snapshot_loaded: bool,
 ) -> v8::ExternalReferences {
-  let mut references = vec![v8::ExternalReference {
-    function: call_console.map_fn_to(),
-  }];
+  let mut references = vec![
+    v8::ExternalReference {
+      function: call_console.map_fn_to(),
+    },
+    v8::ExternalReference {
+      function: import_meta_resolve.map_fn_to(),
+    },
+    v8::ExternalReference {
+      function: catch_dynamic_import_promise_error.map_fn_to(),
+    },
+    v8::ExternalReference {
+      function: empty_fn.map_fn_to(),
+    },
+  ];
 
   for ctx in ops {
     let ctx_ptr = ctx as *const OpCtx as _;
@@ -292,47 +303,11 @@ pub extern "C" fn host_import_module_dynamically_callback(
   // ones rethrown from this scope, so they include the call stack of the
   // dynamic import site. Error objects without any stack frames are assumed to
   // be module resolution errors, other exception values are left as they are.
-  let map_err = |scope: &mut v8::HandleScope,
-                 args: v8::FunctionCallbackArguments,
-                 _rv: v8::ReturnValue| {
-    let arg = args.get(0);
-    if is_instance_of_error(scope, arg) {
-      let e: crate::error::NativeJsError =
-        serde_v8::from_v8(scope, arg).unwrap();
-      let name = e.name.unwrap_or_else(|| "Error".to_string());
-      let message = v8::Exception::create_message(scope, arg);
-      if message.get_stack_trace(scope).unwrap().get_frame_count() == 0 {
-        let arg: v8::Local<v8::Object> = arg.try_into().unwrap();
-        let message_key = v8::String::new(scope, "message").unwrap();
-        let message = arg.get(scope, message_key.into()).unwrap();
-        let exception = match name.as_str() {
-          "RangeError" => {
-            v8::Exception::range_error(scope, message.try_into().unwrap())
-          }
-          "TypeError" => {
-            v8::Exception::type_error(scope, message.try_into().unwrap())
-          }
-          "SyntaxError" => {
-            v8::Exception::syntax_error(scope, message.try_into().unwrap())
-          }
-          "ReferenceError" => {
-            v8::Exception::reference_error(scope, message.try_into().unwrap())
-          }
-          _ => v8::Exception::error(scope, message.try_into().unwrap()),
-        };
-        let code_key = v8::String::new(scope, "code").unwrap();
-        let code_value =
-          v8::String::new(scope, "ERR_MODULE_NOT_FOUND").unwrap();
-        let exception_obj = exception.to_object(scope).unwrap();
-        exception_obj.set(scope, code_key.into(), code_value.into());
-        scope.throw_exception(exception);
-        return;
-      }
-    }
-    scope.throw_exception(arg);
-  };
-  let map_err = v8::FunctionTemplate::new(scope, map_err);
-  let map_err = map_err.get_function(scope).unwrap();
+  let builder = v8::FunctionBuilder::new(catch_dynamic_import_promise_error);
+
+  let map_err =
+    v8::FunctionBuilder::<v8::Function>::build(builder, scope).unwrap();
+
   let promise = promise.catch(scope, map_err).unwrap();
 
   &*promise as *const _ as *mut _
@@ -403,6 +378,62 @@ fn import_meta_resolve(
   };
 }
 
+fn empty_fn(
+  _scope: &mut v8::HandleScope,
+  _args: v8::FunctionCallbackArguments,
+  _rv: v8::ReturnValue,
+) {
+  //Do Nothing
+}
+
+//It creates a reference to an empty function which can be mantained after the snapshots
+pub fn create_empty_fn<'s>(
+  scope: &mut v8::HandleScope<'s>,
+) -> Option<v8::Local<'s, v8::Function>> {
+  let empty_fn = v8::FunctionTemplate::new(scope, empty_fn);
+  empty_fn.get_function(scope)
+}
+
+fn catch_dynamic_import_promise_error(
+  scope: &mut v8::HandleScope,
+  args: v8::FunctionCallbackArguments,
+  _rv: v8::ReturnValue,
+) {
+  let arg = args.get(0);
+  if is_instance_of_error(scope, arg) {
+    let e: crate::error::NativeJsError = serde_v8::from_v8(scope, arg).unwrap();
+    let name = e.name.unwrap_or_else(|| "Error".to_string());
+    let message = v8::Exception::create_message(scope, arg);
+    if message.get_stack_trace(scope).unwrap().get_frame_count() == 0 {
+      let arg: v8::Local<v8::Object> = arg.try_into().unwrap();
+      let message_key = v8::String::new(scope, "message").unwrap();
+      let message = arg.get(scope, message_key.into()).unwrap();
+      let exception = match name.as_str() {
+        "RangeError" => {
+          v8::Exception::range_error(scope, message.try_into().unwrap())
+        }
+        "TypeError" => {
+          v8::Exception::type_error(scope, message.try_into().unwrap())
+        }
+        "SyntaxError" => {
+          v8::Exception::syntax_error(scope, message.try_into().unwrap())
+        }
+        "ReferenceError" => {
+          v8::Exception::reference_error(scope, message.try_into().unwrap())
+        }
+        _ => v8::Exception::error(scope, message.try_into().unwrap()),
+      };
+      let code_key = v8::String::new(scope, "code").unwrap();
+      let code_value = v8::String::new(scope, "ERR_MODULE_NOT_FOUND").unwrap();
+      let exception_obj = exception.to_object(scope).unwrap();
+      exception_obj.set(scope, code_key.into(), code_value.into());
+      scope.throw_exception(exception);
+      return;
+    }
+  }
+  scope.throw_exception(arg);
+}
+
 pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) {
   use v8::PromiseRejectEvent::*;
 

core/modules.rs

@@ -1145,6 +1145,7 @@ mod tests {
   use crate::Extension;
   use crate::JsRuntime;
   use crate::RuntimeOptions;
+  use crate::Snapshot;
   use deno_ops::op;
   use futures::future::FutureExt;
   use parking_lot::Mutex;
@@ -2305,4 +2306,86 @@ if (import.meta.url != 'file:///main_with_code.js') throw Error();
     let _ = runtime.mod_evaluate(side_id);
     futures::executor::block_on(runtime.run_event_loop(false)).unwrap();
   }
+
+  #[test]
+  fn dynamic_imports_snapshot() {
+    //TODO: Once the issue with the ModuleNamespaceEntryGetter is fixed, we can maintain a reference to the module
+    // and use it when loading the snapshot
+    let snapshot = {
+      const MAIN_WITH_CODE_SRC: &str = r#"
+      await import("./b.js");
+    "#;
+
+      let loader = MockLoader::new();
+      let mut runtime = JsRuntime::new(RuntimeOptions {
+        module_loader: Some(loader),
+        will_snapshot: true,
+        ..Default::default()
+      });
+      // In default resolution code should be empty.
+      // Instead we explicitly pass in our own code.
+      // The behavior should be very similar to /a.js.
+      let spec = resolve_url("file:///main_with_code.js").unwrap();
+      let main_id_fut = runtime
+        .load_main_module(&spec, Some(MAIN_WITH_CODE_SRC.to_owned()))
+        .boxed_local();
+      let main_id = futures::executor::block_on(main_id_fut).unwrap();
+
+      let _ = runtime.mod_evaluate(main_id);
+      futures::executor::block_on(runtime.run_event_loop(false)).unwrap();
+      runtime.snapshot()
+    };
+
+    let snapshot = Snapshot::JustCreated(snapshot);
+    let mut runtime2 = JsRuntime::new(RuntimeOptions {
+      startup_snapshot: Some(snapshot),
+      ..Default::default()
+    });
+
+    //Evaluate the snapshot with an empty function
+    runtime2.execute_script("check.js", "true").unwrap();
+  }
+
+  #[test]
+  fn import_meta_snapshot() {
+    let snapshot = {
+      const MAIN_WITH_CODE_SRC: &str = r#"
+    if (import.meta.url != 'file:///main_with_code.js') throw Error();
+    globalThis.meta = import.meta;
+    globalThis.url = import.meta.url;
+    "#;
+
+      let loader = MockLoader::new();
+      let mut runtime = JsRuntime::new(RuntimeOptions {
+        module_loader: Some(loader),
+        will_snapshot: true,
+        ..Default::default()
+      });
+      // In default resolution code should be empty.
+      // Instead we explicitly pass in our own code.
+      // The behavior should be very similar to /a.js.
+      let spec = resolve_url("file:///main_with_code.js").unwrap();
+      let main_id_fut = runtime
+        .load_main_module(&spec, Some(MAIN_WITH_CODE_SRC.to_owned()))
+        .boxed_local();
+      let main_id = futures::executor::block_on(main_id_fut).unwrap();
+
+      let _ = runtime.mod_evaluate(main_id);
+      futures::executor::block_on(runtime.run_event_loop(false)).unwrap();
+      runtime.snapshot()
+    };
+
+    let snapshot = Snapshot::JustCreated(snapshot);
+    let mut runtime2 = JsRuntime::new(RuntimeOptions {
+      startup_snapshot: Some(snapshot),
+      ..Default::default()
+    });
+
+    runtime2
+      .execute_script(
+        "check.js",
+        "if (globalThis.url !== 'file:///main_with_code.js') throw Error('x')",
+      )
+      .unwrap();
+  }
 }

core/runtime.rs

@@ -1283,11 +1283,7 @@ impl JsRuntime {
       );
       let promise = v8::Local::<v8::Promise>::try_from(value)
         .expect("Expected to get promise as module evaluation result");
-      let empty_fn = |_scope: &mut v8::HandleScope,
-                      _args: v8::FunctionCallbackArguments,
-                      _rv: v8::ReturnValue| {};
-      let empty_fn = v8::FunctionTemplate::new(tc_scope, empty_fn);
-      let empty_fn = empty_fn.get_function(tc_scope).unwrap();
+      let empty_fn = bindings::create_empty_fn(tc_scope).unwrap();
       promise.catch(tc_scope, empty_fn);
       let mut state = state_rc.borrow_mut();
       let promise_global = v8::Global::new(tc_scope, promise);