From f840906943849f5a09981e172d57e84301b77386 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 18 Sep 2021 03:44:53 +0200 Subject: [PATCH] fix(core): prevent multiple main module loading (#12128) This commit fixes a problem where loading and executing multiple modules leads to all of the having "import.meta.main" set to true. Following Rust APIs were deprecated: - deno_core::JsRuntime::load_module - deno_runtime::Worker::execute_module - deno_runtime::WebWorker::execute_module Following Rust APIs were added: - deno_core::JsRuntime::load_main_module - deno_core::JsRuntime::load_side_module - deno_runtime::Worker::execute_main_module - deno_runtime::Worker::execute_side_module - deno_runtime::WebWorker::execute_main_module Trying to load multiple "main" modules into the runtime now results in an error. If user needs to load additional "non-main" modules they should use APIs for "side" module. --- cli/main.rs | 10 +-- cli/standalone.rs | 2 +- cli/tools/test.rs | 2 +- core/modules.rs | 131 ++++++++++++++++++++++++++++-- core/runtime.rs | 63 +++++++++++++- runtime/examples/hello_runtime.rs | 2 +- runtime/web_worker.rs | 34 ++++++-- runtime/worker.rs | 64 ++++++++++++--- 8 files changed, 271 insertions(+), 37 deletions(-) diff --git a/cli/main.rs b/cli/main.rs index 672668c2a8..8c27e34b7d 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -492,7 +492,7 @@ async fn install_command( let mut worker = create_main_worker(&program_state, main_module.clone(), permissions, None); // First, fetch and compile the module; this step ensures that the module exists. - worker.preload_module(&main_module).await?; + worker.preload_module(&main_module, true).await?; tools::installer::install( flags, &install_flags.module_url, @@ -604,7 +604,7 @@ async fn eval_command( // to allow module access by TS compiler. program_state.file_fetcher.insert_cached(file); debug!("main_module {}", &main_module); - worker.execute_module(&main_module).await?; + worker.execute_main_module(&main_module).await?; worker.execute_script( &located_script_name!(), "window.dispatchEvent(new Event('load'))", @@ -862,7 +862,7 @@ async fn run_from_stdin(flags: Flags) -> Result<(), AnyError> { program_state.file_fetcher.insert_cached(source_file); debug!("main_module {}", main_module); - worker.execute_module(&main_module).await?; + worker.execute_main_module(&main_module).await?; worker.execute_script( &located_script_name!(), "window.dispatchEvent(new Event('load'))", @@ -949,7 +949,7 @@ async fn run_with_watch(flags: Flags, script: String) -> Result<(), AnyError> { &mut self, main_module: &ModuleSpecifier, ) -> Result<(), AnyError> { - self.worker.execute_module(main_module).await?; + self.worker.execute_main_module(main_module).await?; self.worker.execute_script( &located_script_name!(), "window.dispatchEvent(new Event('load'))", @@ -1044,7 +1044,7 @@ async fn run_command( }; debug!("main_module {}", main_module); - worker.execute_module(&main_module).await?; + worker.execute_main_module(&main_module).await?; worker.execute_script( &located_script_name!(), "window.dispatchEvent(new Event('load'))", diff --git a/cli/standalone.rs b/cli/standalone.rs index a3ace49f7a..19132723ce 100644 --- a/cli/standalone.rs +++ b/cli/standalone.rs @@ -271,7 +271,7 @@ pub async fn run( js_runtime.sync_ops_cache(); } worker.bootstrap(&options); - worker.execute_module(&main_module).await?; + worker.execute_main_module(&main_module).await?; worker.execute_script( &located_script_name!(), "window.dispatchEvent(new Event('load'))", diff --git a/cli/tools/test.rs b/cli/tools/test.rs index a807bde055..0113f1e387 100644 --- a/cli/tools/test.rs +++ b/cli/tools/test.rs @@ -306,7 +306,7 @@ async fn test_specifier( None }; - worker.execute_module(&test_specifier).await?; + worker.execute_main_module(&test_specifier).await?; worker.js_runtime.execute_script( &located_script_name!(), diff --git a/core/modules.rs b/core/modules.rs index f1f083fe9f..a21e974647 100644 --- a/core/modules.rs +++ b/core/modules.rs @@ -186,6 +186,8 @@ impl ModuleLoader for FsModuleLoader { enum LoadInit { /// Main module specifier. Main(String), + /// Module specifier for side module. + Side(String), /// Dynamic import specifier with referrer. DynamicImport(String, String), } @@ -221,6 +223,10 @@ impl RecursiveModuleLoad { Self::new(LoadInit::Main(specifier.to_string()), module_map_rc) } + pub fn side(specifier: &str, module_map_rc: Rc>) -> Self { + Self::new(LoadInit::Side(specifier.to_string()), module_map_rc) + } + pub fn dynamic_import( specifier: &str, referrer: &str, @@ -267,6 +273,11 @@ impl RecursiveModuleLoad { .loader .resolve(self.op_state.clone(), specifier, ".", true) } + LoadInit::Side(ref specifier) => { + self + .loader + .resolve(self.op_state.clone(), specifier, ".", false) + } LoadInit::DynamicImport(ref specifier, ref referrer) => self .loader .resolve(self.op_state.clone(), specifier, referrer, false), @@ -283,6 +294,13 @@ impl RecursiveModuleLoad { .resolve(op_state.clone(), specifier, ".", true)?; (spec, None) } + LoadInit::Side(ref specifier) => { + let spec = + self + .loader + .resolve(op_state.clone(), specifier, ".", false)?; + (spec, None) + } LoadInit::DynamicImport(ref specifier, ref referrer) => { let spec = self @@ -305,7 +323,9 @@ impl RecursiveModuleLoad { } pub fn is_currently_loading_main_module(&self) -> bool { - !self.is_dynamic_import() && self.state == LoadState::LoadingRoot + !self.is_dynamic_import() + && matches!(self.init, LoadInit::Main(..)) + && self.state == LoadState::LoadingRoot } pub fn register_and_recurse( @@ -575,6 +595,17 @@ impl ModuleMap { import_specifiers.push(module_specifier); } + if main { + let maybe_main_module = self.info.values().find(|module| module.main); + if let Some(main_module) = maybe_main_module { + return Err(generic_error( + format!("Trying to create \"main\" module ({:?}), when one already exists ({:?})", + name, + main_module.name, + ))); + } + } + let handle = v8::Global::::new(tc_scope, module); let id = self.next_module_id; self.next_module_id += 1; @@ -644,6 +675,15 @@ impl ModuleMap { Ok(load) } + pub async fn load_side( + module_map_rc: Rc>, + specifier: &str, + ) -> Result { + let load = RecursiveModuleLoad::side(specifier, module_map_rc.clone()); + load.prepare().await?; + Ok(load) + } + // Initiate loading of a module graph imported using `import()`. pub fn load_dynamic_import( module_map_rc: Rc>, @@ -903,7 +943,7 @@ mod tests { ..Default::default() }); let spec = crate::resolve_url("file:///a.js").unwrap(); - let a_id_fut = runtime.load_module(&spec, None); + let a_id_fut = runtime.load_main_module(&spec, None); let a_id = futures::executor::block_on(a_id_fut).expect("Failed to load"); let _ = runtime.mod_evaluate(a_id); @@ -1363,7 +1403,7 @@ mod tests { let fut = async move { let spec = crate::resolve_url("file:///circular1.js").unwrap(); - let result = runtime.load_module(&spec, None).await; + let result = runtime.load_main_module(&spec, None).await; assert!(result.is_ok()); let circular1_id = result.unwrap(); let _ = runtime.mod_evaluate(circular1_id); @@ -1435,7 +1475,7 @@ mod tests { let fut = async move { let spec = crate::resolve_url("file:///redirect1.js").unwrap(); - let result = runtime.load_module(&spec, None).await; + let result = runtime.load_main_module(&spec, None).await; println!(">> result {:?}", result); assert!(result.is_ok()); let redirect1_id = result.unwrap(); @@ -1500,7 +1540,8 @@ mod tests { ..Default::default() }); let spec = crate::resolve_url("file:///main.js").unwrap(); - let mut recursive_load = runtime.load_module(&spec, None).boxed_local(); + let mut recursive_load = + runtime.load_main_module(&spec, None).boxed_local(); let result = recursive_load.poll_unpin(&mut cx); assert!(result.is_pending()); @@ -1548,7 +1589,7 @@ mod tests { ..Default::default() }); let spec = crate::resolve_url("file:///bad_import.js").unwrap(); - let mut load_fut = runtime.load_module(&spec, None).boxed_local(); + let mut load_fut = runtime.load_main_module(&spec, None).boxed_local(); let result = load_fut.poll_unpin(&mut cx); if let Poll::Ready(Err(err)) = result { assert_eq!( @@ -1583,7 +1624,7 @@ mod tests { // The behavior should be very similar to /a.js. let spec = crate::resolve_url("file:///main_with_code.js").unwrap(); let main_id_fut = runtime - .load_module(&spec, Some(MAIN_WITH_CODE_SRC.to_owned())) + .load_main_module(&spec, Some(MAIN_WITH_CODE_SRC.to_owned())) .boxed_local(); let main_id = futures::executor::block_on(main_id_fut).expect("Failed to load"); @@ -1622,4 +1663,80 @@ mod tests { ); assert_eq!(modules.get_children(d_id), Some(&vec![])); } + + #[test] + fn main_and_side_module() { + struct ModsLoader {} + + let main_specifier = crate::resolve_url("file:///main_module.js").unwrap(); + let side_specifier = crate::resolve_url("file:///side_module.js").unwrap(); + + impl ModuleLoader for ModsLoader { + fn resolve( + &self, + _op_state: Rc>, + specifier: &str, + referrer: &str, + _is_main: bool, + ) -> Result { + let s = crate::resolve_import(specifier, referrer).unwrap(); + Ok(s) + } + + fn load( + &self, + _op_state: Rc>, + module_specifier: &ModuleSpecifier, + _maybe_referrer: Option, + _is_dyn_import: bool, + ) -> Pin> { + let module_source = match module_specifier.as_str() { + "file:///main_module.js" => Ok(ModuleSource { + module_url_specified: "file:///main_module.js".to_string(), + module_url_found: "file:///main_module.js".to_string(), + code: "if (!import.meta.main) throw Error();".to_owned(), + }), + "file:///side_module.js" => Ok(ModuleSource { + module_url_specified: "file:///side_module.js".to_string(), + module_url_found: "file:///side_module.js".to_string(), + code: "if (import.meta.main) throw Error();".to_owned(), + }), + _ => unreachable!(), + }; + async move { module_source }.boxed() + } + } + + let loader = Rc::new(ModsLoader {}); + let mut runtime = JsRuntime::new(RuntimeOptions { + module_loader: Some(loader), + ..Default::default() + }); + + let main_id_fut = runtime + .load_main_module(&main_specifier, None) + .boxed_local(); + let main_id = + futures::executor::block_on(main_id_fut).expect("Failed to load"); + + let _ = runtime.mod_evaluate(main_id); + futures::executor::block_on(runtime.run_event_loop(false)).unwrap(); + + // Try to add another main module - it should error. + let side_id_fut = runtime + .load_main_module(&side_specifier, None) + .boxed_local(); + futures::executor::block_on(side_id_fut) + .expect_err("Should have failed to load second main module"); + + // And now try to load it as a side module + let side_id_fut = runtime + .load_side_module(&side_specifier, None) + .boxed_local(); + let side_id = + futures::executor::block_on(side_id_fut).expect("Failed to load"); + + let _ = runtime.mod_evaluate(side_id); + futures::executor::block_on(runtime.run_event_loop(false)).unwrap(); + } } diff --git a/core/runtime.rs b/core/runtime.rs index d5089569d2..f2e804abb1 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -1350,11 +1350,14 @@ impl JsRuntime { state_rc.borrow_mut().pending_dyn_mod_evaluate = still_pending; } - /// Asynchronously load specified module and all of its dependencies + /// Asynchronously load specified module and all of its dependencies. + /// + /// The module will be marked as "main", and because of that + /// "import.meta.main" will return true when checked inside that module. /// /// User must call `JsRuntime::mod_evaluate` with returned `ModuleId` /// manually after load is finished. - pub async fn load_module( + pub async fn load_main_module( &mut self, specifier: &ModuleSpecifier, code: Option, @@ -1363,6 +1366,7 @@ impl JsRuntime { if let Some(code) = code { module_map_rc.borrow_mut().new_module( &mut self.handle_scope(), + // main module true, specifier.as_str(), &code, @@ -1383,6 +1387,59 @@ impl JsRuntime { Ok(root_id) } + /// Asynchronously load specified ES module and all of its dependencies. + /// + /// This method is meant to be used when loading some utility code that + /// might be later imported by the main module (ie. an entry point module). + /// + /// User must call `JsRuntime::mod_evaluate` with returned `ModuleId` + /// manually after load is finished. + pub async fn load_side_module( + &mut self, + specifier: &ModuleSpecifier, + code: Option, + ) -> Result { + let module_map_rc = Self::module_map(self.v8_isolate()); + if let Some(code) = code { + module_map_rc.borrow_mut().new_module( + &mut self.handle_scope(), + // not main module + false, + specifier.as_str(), + &code, + )?; + } + + let mut load = + ModuleMap::load_side(module_map_rc.clone(), specifier.as_str()).await?; + + while let Some(info_result) = load.next().await { + let info = info_result?; + let scope = &mut self.handle_scope(); + load.register_and_recurse(scope, &info)?; + } + + let root_id = load.root_module_id.expect("Root module should be loaded"); + self.instantiate_module(root_id)?; + Ok(root_id) + } + + /// Asynchronously load specified module and all of its dependencies + /// + /// User must call `JsRuntime::mod_evaluate` with returned `ModuleId` + /// manually after load is finished. + #[deprecated( + since = "0.100.0", + note = "This method had a bug, marking multiple modules loaded as \"main\". Use `load_main_module` or `load_side_module` instead." + )] + pub async fn load_module( + &mut self, + specifier: &ModuleSpecifier, + code: Option, + ) -> Result { + self.load_main_module(specifier, code).await + } + fn poll_pending_ops( &mut self, cx: &mut Context, @@ -2040,7 +2097,7 @@ pub mod tests { let source_code = "Deno.core.print('hello\\n')".to_string(); let module_id = futures::executor::block_on( - runtime.load_module(&specifier, Some(source_code)), + runtime.load_main_module(&specifier, Some(source_code)), ) .unwrap(); diff --git a/runtime/examples/hello_runtime.rs b/runtime/examples/hello_runtime.rs index 9ac1d0a273..db63946941 100644 --- a/runtime/examples/hello_runtime.rs +++ b/runtime/examples/hello_runtime.rs @@ -57,7 +57,7 @@ async fn main() -> Result<(), AnyError> { let mut worker = MainWorker::from_options(main_module.clone(), permissions, &options); worker.bootstrap(&options); - worker.execute_module(&main_module).await?; + worker.execute_main_module(&main_module).await?; worker.run_event_loop(false).await?; Ok(()) } diff --git a/runtime/web_worker.rs b/runtime/web_worker.rs index ed7b95f5ba..975028b169 100644 --- a/runtime/web_worker.rs +++ b/runtime/web_worker.rs @@ -458,21 +458,32 @@ impl WebWorker { Ok(()) } - /// Loads and instantiates specified JavaScript module. + /// Loads and instantiates specified JavaScript module + /// as "main" or "side" module. pub async fn preload_module( &mut self, module_specifier: &ModuleSpecifier, + main: bool, ) -> Result { - self.js_runtime.load_module(module_specifier, None).await + if main { + self + .js_runtime + .load_main_module(module_specifier, None) + .await + } else { + self + .js_runtime + .load_side_module(module_specifier, None) + .await + } } /// Loads, instantiates and executes specified JavaScript module. - pub async fn execute_module( + pub async fn execute_main_module( &mut self, module_specifier: &ModuleSpecifier, ) -> Result<(), AnyError> { - let id = self.preload_module(module_specifier).await?; - + let id = self.preload_module(module_specifier, true).await?; let mut receiver = self.js_runtime.mod_evaluate(id); tokio::select! { maybe_result = &mut receiver => { @@ -494,6 +505,17 @@ impl WebWorker { } } + #[deprecated( + since = "0.26.0", + note = "This method had a bug, marking multiple modules loaded as \"main\". Use `execute_main_module`." + )] + pub async fn execute_module( + &mut self, + module_specifier: &ModuleSpecifier, + ) -> Result<(), AnyError> { + self.execute_main_module(module_specifier).await + } + pub fn poll_event_loop( &mut self, cx: &mut Context, @@ -568,7 +590,7 @@ pub fn run_web_worker( } else { // TODO(bartlomieju): add "type": "classic", ie. ability to load // script instead of module - worker.execute_module(&specifier).await + worker.execute_main_module(&specifier).await }; let internal_handle = worker.internal_handle.clone(); diff --git a/runtime/worker.rs b/runtime/worker.rs index 92bebe92a9..54b8a8d684 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -207,21 +207,27 @@ impl MainWorker { Ok(()) } - /// Loads and instantiates specified JavaScript module. + /// Loads and instantiates specified JavaScript module + /// as "main" or "side" module. pub async fn preload_module( &mut self, module_specifier: &ModuleSpecifier, + main: bool, ) -> Result { - self.js_runtime.load_module(module_specifier, None).await + if main { + self + .js_runtime + .load_main_module(module_specifier, None) + .await + } else { + self + .js_runtime + .load_side_module(module_specifier, None) + .await + } } - /// Loads, instantiates and executes specified JavaScript module. - pub async fn execute_module( - &mut self, - module_specifier: &ModuleSpecifier, - ) -> Result<(), AnyError> { - let id = self.preload_module(module_specifier).await?; - self.wait_for_inspector_session(); + async fn evaluate_module(&mut self, id: ModuleId) -> Result<(), AnyError> { let mut receiver = self.js_runtime.mod_evaluate(id); tokio::select! { maybe_result = &mut receiver => { @@ -237,6 +243,38 @@ impl MainWorker { } } + /// Loads, instantiates and executes specified JavaScript module. + pub async fn execute_side_module( + &mut self, + module_specifier: &ModuleSpecifier, + ) -> Result<(), AnyError> { + let id = self.preload_module(module_specifier, false).await?; + self.evaluate_module(id).await + } + + /// Loads, instantiates and executes specified JavaScript module. + /// + /// This module will have "import.meta.main" equal to true. + pub async fn execute_main_module( + &mut self, + module_specifier: &ModuleSpecifier, + ) -> Result<(), AnyError> { + let id = self.preload_module(module_specifier, true).await?; + self.wait_for_inspector_session(); + self.evaluate_module(id).await + } + + #[deprecated( + since = "0.26.0", + note = "This method had a bug, marking multiple modules loaded as \"main\". Use `execute_main_module` or `execute_side_module` instead." + )] + pub async fn execute_module( + &mut self, + module_specifier: &ModuleSpecifier, + ) -> Result<(), AnyError> { + self.execute_main_module(module_specifier).await + } + fn wait_for_inspector_session(&mut self) { if self.should_break_on_first_statement { self @@ -330,7 +368,7 @@ mod tests { let p = test_util::testdata_path().join("esm_imports_a.js"); let module_specifier = resolve_url_or_path(&p.to_string_lossy()).unwrap(); let mut worker = create_test_worker(); - let result = worker.execute_module(&module_specifier).await; + let result = worker.execute_main_module(&module_specifier).await; if let Err(err) = result { eprintln!("execute_mod err {:?}", err); } @@ -347,7 +385,7 @@ mod tests { .join("tests/circular1.js"); let module_specifier = resolve_url_or_path(&p.to_string_lossy()).unwrap(); let mut worker = create_test_worker(); - let result = worker.execute_module(&module_specifier).await; + let result = worker.execute_main_module(&module_specifier).await; if let Err(err) = result { eprintln!("execute_mod err {:?}", err); } @@ -361,7 +399,7 @@ mod tests { // "foo" is not a valid module specifier so this should return an error. let mut worker = create_test_worker(); let module_specifier = resolve_url_or_path("does-not-exist").unwrap(); - let result = worker.execute_module(&module_specifier).await; + let result = worker.execute_main_module(&module_specifier).await; assert!(result.is_err()); } @@ -372,7 +410,7 @@ mod tests { let mut worker = create_test_worker(); let p = test_util::testdata_path().join("001_hello.js"); let module_specifier = resolve_url_or_path(&p.to_string_lossy()).unwrap(); - let result = worker.execute_module(&module_specifier).await; + let result = worker.execute_main_module(&module_specifier).await; assert!(result.is_ok()); } }