Skip to content

Commit

Permalink
fix(realms): Make realm creation not hang with async ops running (den…
Browse files Browse the repository at this point in the history
…oland#99)

The way `JsRuntime::init_extension_js` initialized ESM extensions
without a snapshot was by running the event loop and blocking the
thread ntil there were no more tasks. This works fine for the main
realm, because it is created before any user code can start any async
ops. However, other realms may be created while async ops are running,
which would make realm creation hang until those ops are resolved.

Since extensions are not expected to use top-level await, there would
be no need to run the entire event loop for modules to resolve;
calling `JsRealm::evaluate_pending_module` would be enough. If the
module is not resolved after calling that functions, then it or one of
its dependencies has TLA, in which case we panic.

Since a panic like "TLA is not allowed" would be hard to debug,
especially on codebases with many modules, we use
`ModuleMap::find_stalled_top_level_await` to find at least one of the
TLA locations, and we display it in the panic message.
  • Loading branch information
andreubotella authored Jul 31, 2023
1 parent 2f37bbc commit 58a8f6d
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 5 deletions.
35 changes: 30 additions & 5 deletions core/runtime/jsruntime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -967,11 +967,36 @@ impl JsRuntime {
panic!("{} not present in the module map", specifier)
})
};
let receiver = realm.mod_evaluate(self.v8_isolate(), mod_id);
self.run_event_loop(false).await?;
receiver
.await?
.with_context(|| format!("Couldn't execute '{specifier}'"))?;
let mut receiver = realm.mod_evaluate(self.v8_isolate(), mod_id);
realm.evaluate_pending_module(self.v8_isolate());

// After evaluate_pending_module, if the module isn't fully evaluated
// and the resolver solved, it means the module or one of its imports
// uses TLA.
match receiver.try_recv()? {
Some(result) => {
result
.with_context(|| format!("Couldn't execute '{specifier}'"))?;
}
None => {
// Find the TLA location to display it on the panic.
let location = {
let scope = &mut realm.handle_scope(self.v8_isolate());
let module_map = module_map_rc.borrow();
let messages = module_map.find_stalled_top_level_await(scope);
assert!(!messages.is_empty());
let msg = v8::Local::new(scope, &messages[0]);
let js_error = JsError::from_v8_message(scope, msg);
js_error
.frames
.first()
.unwrap()
.maybe_format_location()
.unwrap()
};
panic!("Top-level await is not allowed in extensions ({location})");
}
}
}

#[cfg(debug_assertions)]
Expand Down
43 changes: 43 additions & 0 deletions core/runtime/tests/jsrealm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -888,3 +888,46 @@ fn preserve_snapshotted_modules() {
fn non_snapshot_preserve_snapshotted_modules() {
generic_preserve_snapshotted_modules_test(false, true)
}

/// Test that creating a realm while there are async ops running in another
/// realm will not block until the async ops are resolved. This used to happen
/// with non-snapshotted ESM extensions.
#[tokio::test]
async fn realm_creation_during_async_op_hang() {
#[op]
async fn op_listen(op_state: Rc<RefCell<OpState>>) -> Result<(), Error> {
let (sender, receiver) = oneshot::channel::<()>();
op_state.borrow_mut().put(sender);
receiver.await?;
Ok(())
}

let extension = Extension {
name: "test_ext",
ops: Cow::Borrowed(&[op_listen::DECL]),
esm_files: Cow::Borrowed(&[ExtensionFileSource {
specifier: "mod:test",
code: ExtensionFileSourceCode::IncludedInBinary(""),
}]),
esm_entry_point: Some("mod:test"),
..Default::default()
};

let mut runtime = JsRuntime::new(RuntimeOptions {
extensions: vec![extension],
..Default::default()
});

// Start the op in the main realm.
{
let promise: v8::Local<v8::Promise> = JsRuntime::eval(
&mut runtime.handle_scope(),
"Deno.core.opAsync('op_listen')",
)
.unwrap();
assert_eq!(promise.state(), v8::PromiseState::Pending);
}

// Create another realm. The test succeeds if this operation doesn't hang.
runtime.create_realm(Default::default()).unwrap();
}
41 changes: 41 additions & 0 deletions core/runtime/tests/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use cooked_waker::Wake;
use cooked_waker::WakeRef;
use deno_ops::op;
use futures::future::poll_fn;
use std::borrow::Cow;
use std::rc::Rc;
use std::sync::atomic::AtomicBool;
use std::sync::atomic::AtomicI8;
Expand Down Expand Up @@ -909,3 +910,43 @@ async fn test_stalled_tla() {
assert_eq!(js_error.frames[0].line_number, Some(1));
assert_eq!(js_error.frames[0].column_number, Some(1));
}

#[tokio::test]
#[should_panic(
expected = "Top-level await is not allowed in extensions (mod:tla:2:1)"
)]
async fn tla_in_esm_extensions_panics() {
#[op]
async fn op_wait(ms: usize) {
tokio::time::sleep(Duration::from_millis(ms as u64)).await
}

let extension = Extension {
name: "test_ext",
ops: Cow::Borrowed(&[op_wait::DECL]),
esm_files: Cow::Borrowed(&[
ExtensionFileSource {
specifier: "mod:tla",
code: ExtensionFileSourceCode::IncludedInBinary(
r#"
await Deno.core.opAsync('op_wait', 0);
export const TEST = "foo";
"#,
),
},
ExtensionFileSource {
specifier: "mod:test",
code: ExtensionFileSourceCode::IncludedInBinary("import 'mod:tla';"),
},
]),
esm_entry_point: Some("mod:test"),
..Default::default()
};

// Panics
let _runtime = JsRuntime::new(RuntimeOptions {
module_loader: Some(Rc::new(StaticModuleLoader::new([]))),
extensions: vec![extension],
..Default::default()
});
}

0 comments on commit 58a8f6d

Please sign in to comment.