Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): prevent multiple main module loading #12128

Merged
merged 2 commits into from
Sep 18, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
add tests
  • Loading branch information
bartlomieju committed Sep 18, 2021
commit b96513a713b39a4ae4bdbe07ff41c308a690521c
10 changes: 5 additions & 5 deletions cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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'))",
Expand Down Expand Up @@ -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'))",
Expand Down Expand Up @@ -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'))",
Expand Down Expand Up @@ -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'))",
Expand Down
2 changes: 1 addition & 1 deletion cli/standalone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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'))",
Expand Down
2 changes: 1 addition & 1 deletion cli/tools/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(),
Expand Down
76 changes: 76 additions & 0 deletions core/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1663,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:https:///main_module.js").unwrap();
let side_specifier = crate::resolve_url("file:https:///side_module.js").unwrap();

impl ModuleLoader for ModsLoader {
fn resolve(
&self,
_op_state: Rc<RefCell<OpState>>,
specifier: &str,
referrer: &str,
_is_main: bool,
) -> Result<ModuleSpecifier, AnyError> {
let s = crate::resolve_import(specifier, referrer).unwrap();
Ok(s)
}

fn load(
&self,
_op_state: Rc<RefCell<OpState>>,
module_specifier: &ModuleSpecifier,
_maybe_referrer: Option<ModuleSpecifier>,
_is_dyn_import: bool,
) -> Pin<Box<ModuleSourceFuture>> {
let module_source = match module_specifier.as_str() {
"file:https:///main_module.js" => Ok(ModuleSource {
module_url_specified: "file:https:///main_module.js".to_string(),
module_url_found: "file:https:///main_module.js".to_string(),
code: "if (!import.meta.main) throw Error();".to_owned(),
}),
"file:https:///side_module.js" => Ok(ModuleSource {
module_url_specified: "file:https:///side_module.js".to_string(),
module_url_found: "file:https:///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();
}
}
2 changes: 1 addition & 1 deletion runtime/examples/hello_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
37 changes: 28 additions & 9 deletions runtime/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,24 +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<ModuleId, AnyError> {
self
.js_runtime
.load_main_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 => {
Expand All @@ -497,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,
Expand Down Expand Up @@ -571,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();
Expand Down
67 changes: 51 additions & 16 deletions runtime/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,24 +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<ModuleId, AnyError> {
self
.js_runtime
.load_main_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 => {
Expand All @@ -240,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
Expand Down Expand Up @@ -333,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);
}
Expand All @@ -350,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);
}
Expand All @@ -364,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());
}

Expand All @@ -375,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());
}
}