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
Next Next commit
fix(core): prevent multiple main module loading
  • Loading branch information
bartlomieju committed Sep 17, 2021
commit 790d5fdf8c6ac8ace6ac4add563617efd9b3e0a4
55 changes: 48 additions & 7 deletions core/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
Expand Down Expand Up @@ -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<RefCell<ModuleMap>>) -> Self {
Self::new(LoadInit::Side(specifier.to_string()), module_map_rc)
}

pub fn dynamic_import(
specifier: &str,
referrer: &str,
Expand Down Expand Up @@ -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),
Expand All @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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::<v8::Module>::new(tc_scope, module);
let id = self.next_module_id;
self.next_module_id += 1;
Expand Down Expand Up @@ -644,6 +675,15 @@ impl ModuleMap {
Ok(load)
}

pub async fn load_side(
module_map_rc: Rc<RefCell<ModuleMap>>,
specifier: &str,
) -> Result<RecursiveModuleLoad, AnyError> {
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<RefCell<ModuleMap>>,
Expand Down Expand Up @@ -903,7 +943,7 @@ mod tests {
..Default::default()
});
let spec = crate::resolve_url("file:https:///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);
Expand Down Expand Up @@ -1363,7 +1403,7 @@ mod tests {

let fut = async move {
let spec = crate::resolve_url("file:https:///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);
Expand Down Expand Up @@ -1435,7 +1475,7 @@ mod tests {

let fut = async move {
let spec = crate::resolve_url("file:https:///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();
Expand Down Expand Up @@ -1500,7 +1540,8 @@ mod tests {
..Default::default()
});
let spec = crate::resolve_url("file:https:///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());
Expand Down Expand Up @@ -1548,7 +1589,7 @@ mod tests {
..Default::default()
});
let spec = crate::resolve_url("file:https:///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!(
Expand Down Expand Up @@ -1583,7 +1624,7 @@ mod tests {
// The behavior should be very similar to /a.js.
let spec = crate::resolve_url("file:https:///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");
Expand Down
63 changes: 60 additions & 3 deletions core/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
Expand All @@ -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,
Expand All @@ -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<String>,
) -> Result<ModuleId, AnyError> {
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<String>,
) -> Result<ModuleId, AnyError> {
self.load_main_module(specifier, code).await
}

fn poll_pending_ops(
&mut self,
cx: &mut Context,
Expand Down Expand Up @@ -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();

Expand Down
5 changes: 4 additions & 1 deletion runtime/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,10 @@ impl WebWorker {
&mut self,
module_specifier: &ModuleSpecifier,
) -> Result<ModuleId, AnyError> {
self.js_runtime.load_module(module_specifier, None).await
self
.js_runtime
.load_main_module(module_specifier, None)
.await
}

/// Loads, instantiates and executes specified JavaScript module.
Expand Down
5 changes: 4 additions & 1 deletion runtime/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,10 @@ impl MainWorker {
&mut self,
module_specifier: &ModuleSpecifier,
) -> Result<ModuleId, AnyError> {
self.js_runtime.load_module(module_specifier, None).await
self
.js_runtime
.load_main_module(module_specifier, None)
.await
}

/// Loads, instantiates and executes specified JavaScript module.
Expand Down