Repository: denoland/deno_core
Pull request #514 (2024-01-29 21:58:53)

refactor: Further cleanups of JsRuntime initialization
Closes https://github.com/denoland/deno_core/issues/502


Patch:
diff --git a/core/extension_set.rs b/core/extension_set.rs
index 03ecfa3..d1a5b28 100644
--- a/core/extension_set.rs
+++ b/core/extension_set.rs
@@ -16,12 +16,7 @@ use crate::OpMetricsFactoryFn;
 use crate::OpState;
 
 /// Contribute to the `OpState` from each extension.
-pub fn setup_op_state(
-  op_state: &mut OpState,
-  deno_core_ext: &mut Extension,
-  extensions: &mut [Extension],
-) {
-  deno_core_ext.take_state(op_state);
+pub fn setup_op_state(op_state: &mut OpState, extensions: &mut [Extension]) {
   for ext in extensions {
     ext.take_state(op_state);
   }
@@ -32,19 +27,19 @@ pub fn setup_op_state(
 // but not added to "ext:core/ops" virtual module.
 /// Collects ops from extensions & applies middleware
 pub fn init_ops(
-  deno_core_ext: &mut Extension,
+  deno_core_ops: &'static [OpDecl],
   extensions: &mut [Extension],
 ) -> Vec<OpDecl> {
   // In debug build verify there that inter-Extension dependencies
   // are setup correctly.
   #[cfg(debug_assertions)]
-  check_extensions_dependencies(deno_core_ext, extensions);
+  check_extensions_dependencies(extensions);
 
   let no_of_ops = extensions
     .iter()
     .map(|e| e.op_count())
     .fold(0, |ext_ops_count, count| count + ext_ops_count);
-  let mut ops = Vec::with_capacity(no_of_ops + deno_core_ext.op_count());
+  let mut ops = Vec::with_capacity(no_of_ops + deno_core_ops.len());
 
   // Collect all middlewares - deno_core extension must not have a middleware!
   let middlewares: Vec<Box<OpMiddlewareFn>> = extensions
@@ -56,11 +51,10 @@ pub fn init_ops(
   let macroware = move |d| middlewares.iter().fold(d, |d, m| m(d));
 
   // Collect ops from all extensions and apply a macroware to each of them.
-  let ext_ops = deno_core_ext.init_ops();
-  for ext_op in ext_ops {
+  for core_op in deno_core_ops {
     ops.push(OpDecl {
-      name: ext_op.name,
-      ..macroware(*ext_op)
+      name: core_op.name,
+      ..macroware(*core_op)
     });
   }
 
@@ -83,14 +77,10 @@ pub fn init_ops(
 
 /// This functions panics if any of the extensions is missing its dependencies.
 #[cfg(debug_assertions)]
-fn check_extensions_dependencies(
-  deno_core_ext: &Extension,
-  exts: &[Extension],
-) {
+fn check_extensions_dependencies(exts: &[Extension]) {
   for (index, ext) in exts.iter().enumerate() {
-    let mut previous_exts = vec![deno_core_ext];
-    previous_exts.extend_from_slice(&exts[..index].iter().collect::<Vec<_>>());
-    ext.check_dependencies(&previous_exts);
+    let previous_exts = &exts[..index];
+    ext.check_dependencies(previous_exts);
   }
 }
 
diff --git a/core/extensions.rs b/core/extensions.rs
index 22088a9..e5f43f7 100644
--- a/core/extensions.rs
+++ b/core/extensions.rs
@@ -554,7 +554,7 @@ impl Extension {
   /// Check if dependencies have been loaded, and errors if either:
   /// - The extension is depending on itself or an extension with the same name.
   /// - A dependency hasn't been loaded yet.
-  pub fn check_dependencies(&self, previous_exts: &[&Extension]) {
+  pub fn check_dependencies(&self, previous_exts: &[Extension]) {
     'dep_loop: for dep in self.deps {
       if dep == &self.name {
         panic!("Extension '{}' is either depending on itself or there is another extension with the same name", self.name);
diff --git a/core/modules/map.rs b/core/modules/map.rs
index 360504d..bd0413e 100644
--- a/core/modules/map.rs
+++ b/core/modules/map.rs
@@ -177,19 +177,15 @@ impl ModuleMap {
     }
   }
 
-  pub(crate) fn new_from_snapshotted_data(
-    loader: Rc<dyn ModuleLoader>,
-    exception_state: Rc<ExceptionState>,
-    import_meta_resolve_cb: ImportMetaResolveCallback,
+  pub(crate) fn update_with_snapshotted_data(
+    &self,
     scope: &mut v8::HandleScope,
     data: ModuleMapSnapshottedData,
-  ) -> Self {
-    let new = Self::new(loader, exception_state, import_meta_resolve_cb);
-    new
+  ) {
+    self
       .data
       .borrow_mut()
       .update_with_snapshotted_data(scope, data);
-    new
   }
 
   /// Get module id, following all aliases in case of module specifier
diff --git a/core/ops_builtin.rs b/core/ops_builtin.rs
index 293ca78..abfa217 100644
--- a/core/ops_builtin.rs
+++ b/core/ops_builtin.rs
@@ -9,6 +9,8 @@ use crate::op2;
 use crate::ops_builtin_types;
 use crate::ops_builtin_v8;
 use crate::JsBuffer;
+use crate::Op;
+use crate::OpDecl;
 use crate::OpState;
 use crate::Resource;
 use anyhow::Error;
@@ -19,97 +21,102 @@ use std::io::stdout;
 use std::io::Write;
 use std::rc::Rc;
 
-deno_core::extension!(
-  core,
-  ops = [
-    op_close,
-    op_try_close,
-    op_print,
-    op_resources,
-    op_wasm_streaming_feed,
-    op_wasm_streaming_set_url,
-    op_void_sync,
-    op_error_async,
-    op_error_async_deferred,
-    op_void_async,
-    op_void_async_deferred,
-    op_add,
-    op_add_async,
-    op_read,
-    op_read_all,
-    op_write,
-    op_read_sync,
-    op_write_sync,
-    op_write_all,
-    op_write_type_error,
-    op_shutdown,
-    op_format_file_name,
-    op_str_byte_length,
-    op_panic,
-    ops_builtin_types::op_is_any_array_buffer,
-    ops_builtin_types::op_is_arguments_object,
-    ops_builtin_types::op_is_array_buffer,
-    ops_builtin_types::op_is_array_buffer_view,
-    ops_builtin_types::op_is_async_function,
-    ops_builtin_types::op_is_big_int_object,
-    ops_builtin_types::op_is_boolean_object,
-    ops_builtin_types::op_is_boxed_primitive,
-    ops_builtin_types::op_is_data_view,
-    ops_builtin_types::op_is_date,
-    ops_builtin_types::op_is_generator_function,
-    ops_builtin_types::op_is_generator_object,
-    ops_builtin_types::op_is_map,
-    ops_builtin_types::op_is_map_iterator,
-    ops_builtin_types::op_is_module_namespace_object,
-    ops_builtin_types::op_is_native_error,
-    ops_builtin_types::op_is_number_object,
-    ops_builtin_types::op_is_promise,
-    ops_builtin_types::op_is_proxy,
-    ops_builtin_types::op_is_reg_exp,
-    ops_builtin_types::op_is_set,
-    ops_builtin_types::op_is_set_iterator,
-    ops_builtin_types::op_is_shared_array_buffer,
-    ops_builtin_types::op_is_string_object,
-    ops_builtin_types::op_is_symbol_object,
-    ops_builtin_types::op_is_typed_array,
-    ops_builtin_types::op_is_weak_map,
-    ops_builtin_types::op_is_weak_set,
-    ops_builtin_v8::op_set_handled_promise_rejection_handler,
-    ops_builtin_v8::op_timer_queue,
-    ops_builtin_v8::op_timer_cancel,
-    ops_builtin_v8::op_timer_ref,
-    ops_builtin_v8::op_timer_unref,
-    ops_builtin_v8::op_ref_op,
-    ops_builtin_v8::op_unref_op,
-    ops_builtin_v8::op_lazy_load_esm,
-    ops_builtin_v8::op_run_microtasks,
-    ops_builtin_v8::op_has_tick_scheduled,
-    ops_builtin_v8::op_set_has_tick_scheduled,
-    ops_builtin_v8::op_eval_context,
-    ops_builtin_v8::op_queue_microtask,
-    ops_builtin_v8::op_encode,
-    ops_builtin_v8::op_decode,
-    ops_builtin_v8::op_serialize,
-    ops_builtin_v8::op_deserialize,
-    ops_builtin_v8::op_set_promise_hooks,
-    ops_builtin_v8::op_get_promise_details,
-    ops_builtin_v8::op_get_proxy_details,
-    ops_builtin_v8::op_get_non_index_property_names,
-    ops_builtin_v8::op_get_constructor_name,
-    ops_builtin_v8::op_memory_usage,
-    ops_builtin_v8::op_set_wasm_streaming_callback,
-    ops_builtin_v8::op_abort_wasm_streaming,
-    ops_builtin_v8::op_destructure_error,
-    ops_builtin_v8::op_dispatch_exception,
-    ops_builtin_v8::op_op_names,
-    ops_builtin_v8::op_apply_source_map,
-    ops_builtin_v8::op_apply_source_map_filename,
-    ops_builtin_v8::op_current_user_call_site,
-    ops_builtin_v8::op_set_format_exception_callback,
-    ops_builtin_v8::op_event_loop_has_more_work,
-    ops_builtin_v8::op_arraybuffer_was_detached,
-  ],
-);
+macro_rules! builtin_ops {
+  ( $($op:ident $(:: $sub:ident)*),* ) => {
+    pub const BUILTIN_OPS: &'static [OpDecl] = &[
+      $( $op $(:: $sub)*::DECL, )*
+    ];
+  }
+}
+
+builtin_ops! {
+  op_close,
+  op_try_close,
+  op_print,
+  op_resources,
+  op_wasm_streaming_feed,
+  op_wasm_streaming_set_url,
+  op_void_sync,
+  op_error_async,
+  op_error_async_deferred,
+  op_void_async,
+  op_void_async_deferred,
+  op_add,
+  op_add_async,
+  op_read,
+  op_read_all,
+  op_write,
+  op_read_sync,
+  op_write_sync,
+  op_write_all,
+  op_write_type_error,
+  op_shutdown,
+  op_format_file_name,
+  op_str_byte_length,
+  op_panic,
+  ops_builtin_types::op_is_any_array_buffer,
+  ops_builtin_types::op_is_arguments_object,
+  ops_builtin_types::op_is_array_buffer,
+  ops_builtin_types::op_is_array_buffer_view,
+  ops_builtin_types::op_is_async_function,
+  ops_builtin_types::op_is_big_int_object,
+  ops_builtin_types::op_is_boolean_object,
+  ops_builtin_types::op_is_boxed_primitive,
+  ops_builtin_types::op_is_data_view,
+  ops_builtin_types::op_is_date,
+  ops_builtin_types::op_is_generator_function,
+  ops_builtin_types::op_is_generator_object,
+  ops_builtin_types::op_is_map,
+  ops_builtin_types::op_is_map_iterator,
+  ops_builtin_types::op_is_module_namespace_object,
+  ops_builtin_types::op_is_native_error,
+  ops_builtin_types::op_is_number_object,
+  ops_builtin_types::op_is_promise,
+  ops_builtin_types::op_is_proxy,
+  ops_builtin_types::op_is_reg_exp,
+  ops_builtin_types::op_is_set,
+  ops_builtin_types::op_is_set_iterator,
+  ops_builtin_types::op_is_shared_array_buffer,
+  ops_builtin_types::op_is_string_object,
+  ops_builtin_types::op_is_symbol_object,
+  ops_builtin_types::op_is_typed_array,
+  ops_builtin_types::op_is_weak_map,
+  ops_builtin_types::op_is_weak_set,
+  ops_builtin_v8::op_set_handled_promise_rejection_handler,
+  ops_builtin_v8::op_timer_queue,
+  ops_builtin_v8::op_timer_cancel,
+  ops_builtin_v8::op_timer_ref,
+  ops_builtin_v8::op_timer_unref,
+  ops_builtin_v8::op_ref_op,
+  ops_builtin_v8::op_unref_op,
+  ops_builtin_v8::op_lazy_load_esm,
+  ops_builtin_v8::op_run_microtasks,
+  ops_builtin_v8::op_has_tick_scheduled,
+  ops_builtin_v8::op_set_has_tick_scheduled,
+  ops_builtin_v8::op_eval_context,
+  ops_builtin_v8::op_queue_microtask,
+  ops_builtin_v8::op_encode,
+  ops_builtin_v8::op_decode,
+  ops_builtin_v8::op_serialize,
+  ops_builtin_v8::op_deserialize,
+  ops_builtin_v8::op_set_promise_hooks,
+  ops_builtin_v8::op_get_promise_details,
+  ops_builtin_v8::op_get_proxy_details,
+  ops_builtin_v8::op_get_non_index_property_names,
+  ops_builtin_v8::op_get_constructor_name,
+  ops_builtin_v8::op_memory_usage,
+  ops_builtin_v8::op_set_wasm_streaming_callback,
+  ops_builtin_v8::op_abort_wasm_streaming,
+  ops_builtin_v8::op_destructure_error,
+  ops_builtin_v8::op_dispatch_exception,
+  ops_builtin_v8::op_op_names,
+  ops_builtin_v8::op_apply_source_map,
+  ops_builtin_v8::op_apply_source_map_filename,
+  ops_builtin_v8::op_current_user_call_site,
+  ops_builtin_v8::op_set_format_exception_callback,
+  ops_builtin_v8::op_event_loop_has_more_work,
+  ops_builtin_v8::op_arraybuffer_was_detached
+}
 
 #[op2(fast)]
 pub fn op_panic(#[string] message: String) {
diff --git a/core/runtime/bindings.rs b/core/runtime/bindings.rs
index 5bdff5c..8a7e860 100644
--- a/core/runtime/bindings.rs
+++ b/core/runtime/bindings.rs
@@ -1,4 +1,5 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+use anyhow::Context;
 use log::debug;
 use std::option::Option;
 use std::os::raw::c_void;
@@ -10,6 +11,7 @@ use super::jsruntime::CONTEXT_SETUP_SOURCES;
 use crate::error::has_call_site;
 use crate::error::is_instance_of_error;
 use crate::error::throw_type_error;
+use crate::error::AnyError;
 use crate::error::JsStackFrame;
 use crate::modules::get_requested_module_type_from_attributes;
 use crate::modules::parse_import_attributes;
@@ -221,42 +223,36 @@ pub(crate) fn initialize_deno_core_namespace<'s>(
   global.set(scope, deno_str.into(), deno_obj.into());
 }
 
-/// If required, execute required JavaScript for `deno_core` to function and
-/// set up JavaScript bindings for ops.
-pub(crate) fn initialize_context<'s>(
+/// Execute `00_primordials.js` and `00_infra.js` that are required for ops
+/// to function properly
+pub(crate) fn initialize_primordials_and_infra(
+  scope: &mut v8::HandleScope,
+) -> Result<(), AnyError> {
+  for file_source in CONTEXT_SETUP_SOURCES {
+    let code = file_source.load().unwrap();
+    let source_str = code.v8_string(scope).unwrap();
+    let name = v8::String::new_external_onebyte_static(
+      scope,
+      file_source.specifier.as_bytes(),
+    )
+    .unwrap();
+    let origin = script_origin(scope, name);
+    // TODO(bartlomieju): these two calls will panic if there's any problem in the JS code
+    let script = v8::Script::compile(scope, source_str, Some(&origin))
+      .with_context(|| format!("Failed to parse {}", file_source.specifier))?;
+    script.run(scope).with_context(|| {
+      format!("Failed to execute {}", file_source.specifier)
+    })?;
+  }
+  Ok(())
+}
+
+/// Set up JavaScript bindings for ops.
+pub(crate) fn initialize_deno_core_ops_bindings<'s>(
   scope: &mut v8::HandleScope<'s>,
   context: v8::Local<'s, v8::Context>,
   op_ctxs: &[OpCtx],
-  init_mode: InitMode,
 ) {
-  // Execute `00_primordials.js` and `00_infra.js`
-  if init_mode == InitMode::New {
-    for file_source in CONTEXT_SETUP_SOURCES {
-      let code = file_source.load().unwrap();
-      let source_str = code.v8_string(scope).unwrap();
-      let name = v8::String::new_external_onebyte_static(
-        scope,
-        file_source.specifier.as_bytes(),
-      )
-      .unwrap();
-      let origin = script_origin(scope, name);
-      // TODO(bartlomieju): these two calls will panic if there's any problem in the JS code
-      let script =
-        v8::Script::compile(scope, source_str, Some(&origin)).unwrap();
-      script.run(scope).unwrap();
-    }
-  }
-
-  // Fast path - if all the ops have been registered already, bail out.
-  if matches!(
-    init_mode,
-    InitMode::FromSnapshot {
-      skip_op_registration: true
-    }
-  ) {
-    return;
-  }
-
   let global = context.global(scope);
 
   // Set up JavaScript bindings for the defined op - this will insert proper
diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs
index 0c6d79b..7da3e5d 100644
--- a/core/runtime/jsruntime.rs
+++ b/core/runtime/jsruntime.rs
@@ -40,6 +40,7 @@ use crate::source_map::SourceMapCache;
 use crate::source_map::SourceMapGetter;
 use crate::Extension;
 use crate::ExtensionFileSource;
+use crate::ExtensionFileSourceCode;
 use crate::FastString;
 use crate::FeatureChecker;
 use crate::NoopModuleLoader;
@@ -220,6 +221,16 @@ impl InitMode {
   pub fn is_from_snapshot(&self) -> bool {
     matches!(self, Self::FromSnapshot { .. })
   }
+
+  #[inline]
+  pub fn needs_ops_bindings(&self) -> bool {
+    !matches!(
+      self,
+      InitMode::FromSnapshot {
+        skip_op_registration: true
+      }
+    )
+  }
 }
 
 #[derive(Default)]
@@ -294,7 +305,10 @@ pub(crate) const NO_OF_BUILTIN_MODULES: usize = 2;
 pub struct JsRuntime {
   pub(crate) inner: InnerIsolateState,
   pub(crate) allocations: IsolateAllocations,
-  extensions: Vec<Extension>,
+  // Contains paths of source files that were executed in
+  // [`JsRuntime::init_extension_js`]. This field is populated only if a
+  // snapshot is being created.
+  files_loaded_from_fs_during_snapshot: Vec<&'static str>,
   init_mode: InitMode,
   // Marks if this is considered the top-level runtime. Used only by inspector.
   is_main_runtime: bool,
@@ -550,7 +564,12 @@ impl JsRuntime {
       cfg!(test),
       options.unsafe_expose_natives_and_gc(),
     );
-    JsRuntime::new_inner(options, false)
+    match JsRuntime::new_inner(options, false) {
+      Ok(runtime) => runtime,
+      Err(err) => {
+        panic!("Failed to initialize a JsRuntime: {:?}", err);
+      }
+    }
   }
 
   pub(crate) fn state_from(isolate: &v8::Isolate) -> Rc<JsRuntimeState> {
@@ -574,20 +593,19 @@ impl JsRuntime {
     EventLoopPendingState::new_from_scope(scope).is_pending()
   }
 
-  fn new_inner(mut options: RuntimeOptions, will_snapshot: bool) -> JsRuntime {
+  fn new_inner(
+    mut options: RuntimeOptions,
+    will_snapshot: bool,
+  ) -> Result<JsRuntime, Error> {
     let init_mode = InitMode::from_options(&options);
     let mut op_state = OpState::new(options.feature_checker.take());
 
     let mut extensions = std::mem::take(&mut options.extensions);
-    let mut deno_core_ext = crate::ops_builtin::core::init_ops();
-    let op_decls = extension_set::init_ops(&mut deno_core_ext, &mut extensions);
-    extension_set::setup_op_state(
-      &mut op_state,
-      &mut deno_core_ext,
-      &mut extensions,
-    );
+    let op_decls =
+      extension_set::init_ops(crate::ops_builtin::BUILTIN_OPS, &mut extensions);
+    extension_set::setup_op_state(&mut op_state, &mut extensions);
     let (
-      global_template_middlewares,
+      global_template_middleware,
       global_object_middlewares,
       additional_references,
     ) = extension_set::get_middlewares_and_external_refs(&mut extensions);
@@ -684,23 +702,23 @@ impl JsRuntime {
 
     // TODO(bartlomieju): this context creation and setup is really non-trivial.
     // Additionally it's unclear what is done when snapshotting/running from snapshot.
-    let (main_context, snapshotted_data) = {
+    let mut snapshotted_data = None;
+    let main_context = {
       let scope = &mut v8::HandleScope::new(&mut isolate);
 
       let context = create_context(
         scope,
-        &global_template_middlewares,
+        &global_template_middleware,
         &global_object_middlewares,
       );
 
       // Get module map data from the snapshot
-      let snapshotted_data = if init_mode.is_from_snapshot() {
-        Some(snapshot_util::get_snapshotted_data(scope, context))
-      } else {
-        None
-      };
+      if init_mode.is_from_snapshot() {
+        snapshotted_data =
+          Some(snapshot_util::get_snapshotted_data(scope, context));
+      }
 
-      (v8::Global::new(scope, context), snapshotted_data)
+      v8::Global::new(scope, context)
     };
 
     let mut context_scope: v8::HandleScope =
@@ -708,13 +726,19 @@ impl JsRuntime {
     let scope = &mut context_scope;
     let context = v8::Local::new(scope, &main_context);
 
-    bindings::initialize_deno_core_namespace(scope, context, init_mode);
-    bindings::initialize_context(
-      scope,
-      context,
-      &context_state.op_ctxs,
-      init_mode,
-    );
+    if init_mode == InitMode::New {
+      bindings::initialize_deno_core_namespace(scope, context, init_mode);
+      bindings::initialize_primordials_and_infra(scope)?;
+    }
+    // If we're creating a new runtime or there are new ops to register
+    // set up JavaScript bindings for them.
+    if init_mode.needs_ops_bindings() {
+      bindings::initialize_deno_core_ops_bindings(
+        scope,
+        context,
+        &context_state.op_ctxs,
+      );
+    }
 
     context.set_slot(scope, context_state.clone());
 
@@ -730,29 +754,24 @@ impl JsRuntime {
     let import_meta_resolve_cb = options
       .import_meta_resolve_callback
       .unwrap_or_else(|| Box::new(default_import_meta_resolve_cb));
-
     let exception_state = context_state.exception_state.clone();
-    let module_map = if let Some(snapshotted_data) = snapshotted_data {
+
+    let module_map = Rc::new(ModuleMap::new(
+      loader,
+      exception_state.clone(),
+      import_meta_resolve_cb,
+    ));
+
+    if let Some(snapshotted_data) = snapshotted_data {
       *exception_state.js_handled_promise_rejection_cb.borrow_mut() =
         snapshotted_data.js_handled_promise_rejection_cb;
       let module_map_snapshotted_data = ModuleMapSnapshottedData {
         module_map_data: snapshotted_data.module_map_data,
         module_handles: snapshotted_data.module_handles,
       };
-      Rc::new(ModuleMap::new_from_snapshotted_data(
-        loader,
-        exception_state,
-        import_meta_resolve_cb,
-        scope,
-        module_map_snapshotted_data,
-      ))
-    } else {
-      Rc::new(ModuleMap::new(
-        loader,
-        exception_state,
-        import_meta_resolve_cb,
-      ))
-    };
+      module_map
+        .update_with_snapshotted_data(scope, module_map_snapshotted_data);
+    }
 
     context.set_slot(scope, module_map.clone());
 
@@ -782,22 +801,19 @@ impl JsRuntime {
       },
       init_mode,
       allocations: IsolateAllocations::default(),
-      // TODO(bartlomieju): at this point extensions have only JS/ESM sources left;
-      // probably worth to add a dedicated struct to represent that.
-      extensions: vec![],
+      files_loaded_from_fs_during_snapshot: vec![],
       is_main_runtime: options.is_main,
     };
 
-    // TODO(mmastrac): We should thread errors back out of the runtime
-    js_runtime
-      .init_extension_js(&mut extensions)
-      .expect("Failed to evaluate extension JS");
+    let files_loaded_from_fs_during_snapshot =
+      js_runtime.init_extension_js(extensions)?;
 
-    // TODO(bartlomieju): clean this up - we shouldn't need to store extensions
-    // on the `js_runtime` as they are mutable.
-    js_runtime.extensions = extensions;
+    if will_snapshot {
+      js_runtime.files_loaded_from_fs_during_snapshot =
+        files_loaded_from_fs_during_snapshot;
+    }
 
-    js_runtime
+    Ok(js_runtime)
   }
 
   #[cfg(test)]
@@ -850,10 +866,8 @@ impl JsRuntime {
     }
   }
 
-  // TODO(bartlomieju): this method probably be public to the crate
-  /// Returns the extensions that this runtime is using (including internal ones).
-  pub fn extensions(&self) -> &Vec<Extension> {
-    &self.extensions
+  pub(crate) fn files_loaded_from_fs_during_snapshot(&self) -> &[&'static str] {
+    &self.files_loaded_from_fs_during_snapshot
   }
 
   #[inline]
@@ -907,59 +921,85 @@ impl JsRuntime {
     module_map.mod_evaluate_sync(scope, mod_id).unwrap();
   }
 
+  /// Executes built-in scripts and ES modules - this code is required for
+  /// ops system to work properly, as well as providing necessary bindings
+  /// on the `Deno.core` namespace.
+  ///
+  /// This is not done in [`bindings::initialize_primordials_and_infra`] because
+  /// some of this code already relies on certain ops being available.
+  fn execute_builtin_sources(
+    &mut self,
+    realm: &JsRealm,
+    module_map: &Rc<ModuleMap>,
+    files_loaded: &mut Vec<&'static str>,
+  ) -> Result<(), Error> {
+    for file_source in &BUILTIN_SOURCES {
+      mark_as_loaded_from_fs_during_snapshot(files_loaded, &file_source.code);
+      realm.execute_script(
+        self.v8_isolate(),
+        file_source.specifier,
+        file_source.load()?,
+      )?;
+    }
+
+    for file_source in &BUILTIN_ES_MODULES {
+      mark_as_loaded_from_fs_during_snapshot(files_loaded, &file_source.code);
+      let mut scope = realm.handle_scope(self.v8_isolate());
+      module_map.lazy_load_es_module_from_code(
+        &mut scope,
+        file_source.specifier,
+        file_source.load()?,
+      )?;
+    }
+
+    Ok(())
+  }
+
   /// Initializes JS of provided Extensions in the given realm.
   fn init_extension_js(
     &mut self,
-    extensions: &mut [Extension],
-  ) -> Result<(), Error> {
+    extensions: Vec<Extension>,
+  ) -> Result<Vec<&'static str>, Error> {
     // Initialization of JS happens in phases:
     // 1. Iterate through all extensions:
     //  a. Execute all extension "script" JS files
     //  b. Load all extension "module" JS files (but do not execute them yet)
     // 2. Iterate through all extensions:
     //  a. If an extension has a `esm_entry_point`, execute it.
+    let mut files_loaded = Vec::with_capacity(128);
     let realm = JsRealm::clone(&self.inner.main_realm);
     let context_global = realm.context();
     let module_map = realm.0.module_map();
+
+    // TODO(bartlomieju): hoist it out of this method.
+    // TODO(bartlomieju): this is somewhat duplicated in `bindings::initialize_context`,
+    // but for migration period we need to have ops available in both `Deno.core.ops`
+    // as well as have them available in "virtual ops module"
+    // if !matches!(
+    //   self.init_mode,
+    //   InitMode::FromSnapshot {
+    //     skip_op_registration: true
+    //   }
+    // ) {
+    if self.init_mode == InitMode::New {
+      self.execute_virtual_ops_module(context_global, module_map.clone());
+    }
+
+    // TODO(bartlomieju): hoist it out of this method.
+    if self.init_mode == InitMode::New {
+      self.execute_builtin_sources(&realm, &module_map, &mut files_loaded)?;
+    }
+
+    // TODO(bartlomieju): hoist it out of this method.
+    self.init_cbs(&realm);
+
     let loader = module_map.loader.borrow().clone();
-    let ext_loader = Rc::new(ExtModuleLoader::new(extensions));
+    let ext_loader = Rc::new(ExtModuleLoader::new(&extensions));
     *module_map.loader.borrow_mut() = ext_loader;
 
     let mut esm_entrypoints = vec![];
 
     futures::executor::block_on(async {
-      // TODO(bartlomieju): this is somewhat duplicated in `bindings::initialize_context`,
-      // but for migration period we need to have ops available in both `Deno.core.ops`
-      // as well as have them available in "virtual ops module"
-      // if !matches!(
-      //   self.init_mode,
-      //   InitMode::FromSnapshot {
-      //     skip_op_registration: true
-      //   }
-      // ) {
-      if self.init_mode == InitMode::New {
-        self.execute_virtual_ops_module(context_global, module_map.clone());
-      }
-
-      if self.init_mode == InitMode::New {
-        for file_source in &BUILTIN_SOURCES {
-          realm.execute_script(
-            self.v8_isolate(),
-            file_source.specifier,
-            file_source.load()?,
-          )?;
-        }
-        for file_source in &BUILTIN_ES_MODULES {
-          let mut scope = realm.handle_scope(self.v8_isolate());
-          module_map.lazy_load_es_module_from_code(
-            &mut scope,
-            file_source.specifier,
-            file_source.load()?,
-          )?;
-        }
-      }
-      self.init_cbs(&realm);
-
       for extension in extensions {
         // If the extension provides "lazy loaded ES modules" then store them
         // on the ModuleMap.
@@ -969,6 +1009,10 @@ impl JsRuntime {
         let maybe_esm_entry_point = extension.get_esm_entry_point();
 
         for file_source in extension.get_esm_sources() {
+          mark_as_loaded_from_fs_during_snapshot(
+            &mut files_loaded,
+            &file_source.code,
+          );
           realm
             .load_side_module(
               self.v8_isolate(),
@@ -983,6 +1027,10 @@ impl JsRuntime {
         }
 
         for file_source in extension.get_js_sources() {
+          mark_as_loaded_from_fs_during_snapshot(
+            &mut files_loaded,
+            &file_source.code,
+          );
           realm.execute_script(
             self.v8_isolate(),
             file_source.specifier,
@@ -1016,7 +1064,7 @@ impl JsRuntime {
     let module_map = realm.0.module_map();
     *module_map.loader.borrow_mut() = loader;
 
-    Ok(())
+    Ok(files_loaded)
   }
 
   pub fn eval<'s, T>(
@@ -1712,7 +1760,14 @@ impl JsRuntimeForSnapshot {
       true,
       options.unsafe_expose_natives_and_gc(),
     );
-    JsRuntimeForSnapshot(JsRuntime::new_inner(options, true))
+
+    let runtime = match JsRuntime::new_inner(options, true) {
+      Ok(r) => r,
+      Err(err) => {
+        panic!("Failed to initialize JsRuntime for snapshotting: {:?}", err);
+      }
+    };
+    JsRuntimeForSnapshot(runtime)
   }
 
   /// Takes a snapshot and consumes the runtime.
@@ -2132,3 +2187,12 @@ impl JsRuntime {
     Ok(dispatched_ops)
   }
 }
+
+fn mark_as_loaded_from_fs_during_snapshot(
+  files_loaded: &mut Vec<&'static str>,
+  source: &ExtensionFileSourceCode,
+) {
+  if let ExtensionFileSourceCode::LoadedFromFsDuringSnapshot(path) = source {
+    files_loaded.push(path);
+  }
+}
diff --git a/core/runtime/snapshot_util.rs b/core/runtime/snapshot_util.rs
index 06e312f..4ac64e6 100644
--- a/core/runtime/snapshot_util.rs
+++ b/core/runtime/snapshot_util.rs
@@ -4,10 +4,7 @@ use std::path::Path;
 use std::path::PathBuf;
 use std::time::Instant;
 
-use crate::runtime::jsruntime::BUILTIN_ES_MODULES;
-use crate::runtime::jsruntime::BUILTIN_SOURCES;
 use crate::Extension;
-use crate::ExtensionFileSourceCode;
 use crate::JsRuntimeForSnapshot;
 use crate::RuntimeOptions;
 use crate::Snapshot;
@@ -50,33 +47,11 @@ pub fn create_snapshot(
   );
   mark = Instant::now();
 
-  let mut files_loaded_during_snapshot = vec![];
-  for source in &BUILTIN_SOURCES {
-    if let ExtensionFileSourceCode::LoadedFromFsDuringSnapshot(path) =
-      &source.code
-    {
-      files_loaded_during_snapshot.push(PathBuf::from(path));
-    }
-  }
-  for source in &BUILTIN_ES_MODULES {
-    if let ExtensionFileSourceCode::LoadedFromFsDuringSnapshot(path) =
-      &source.code
-    {
-      files_loaded_during_snapshot.push(PathBuf::from(path));
-    }
-  }
-  for source in js_runtime
-    .extensions()
+  let files_loaded_during_snapshot = js_runtime
+    .files_loaded_from_fs_during_snapshot()
     .iter()
-    .flat_map(|e| vec![e.get_esm_sources(), e.get_js_sources()])
-    .flatten()
-  {
-    if let ExtensionFileSourceCode::LoadedFromFsDuringSnapshot(path) =
-      &source.code
-    {
-      files_loaded_during_snapshot.push(PathBuf::from(path));
-    }
-  }
+    .map(PathBuf::from)
+    .collect::<Vec<_>>();
 
   if let Some(with_runtime_cb) = create_snapshot_options.with_runtime_cb {
     with_runtime_cb(&mut js_runtime);


Test patch:
diff --git a/core/runtime/tests/misc.rs b/core/runtime/tests/misc.rs
index 156f68a..36e453d 100644
--- a/core/runtime/tests/misc.rs
+++ b/core/runtime/tests/misc.rs
@@ -1010,7 +1010,7 @@ async fn test_dynamic_import_module_error_stack() {
 
 #[tokio::test]
 #[should_panic(
-  expected = r#"Failed to evaluate extension JS: Top-level await is not allowed in extensions
+  expected = r#"Failed to initialize a JsRuntime: Top-level await is not allowed in extensions
 
 Caused by:
     Top-level await promise never resolved
@@ -1057,7 +1057,7 @@ async fn tla_in_esm_extensions_panics() {
 #[cfg(debug_assertions)]
 #[tokio::test]
 #[should_panic(
-  expected = r#"Failed to evaluate extension JS: Error: This fails
+  expected = r#"Failed to initialize a JsRuntime: Error: This fails
     at a (mod:error:2:34)
     at mod:error:3:13"#
 )]