Skip to content

Commit

Permalink
fix(core): Re-run event loop after resolving ops
Browse files Browse the repository at this point in the history

Repro and fix for denoland/deno#19903 and denoland/deno#19455.

No perf impact seen in deno before and after.
  • Loading branch information
mmastrac committed Jul 25, 2023
1 parent ecd8242 commit 39edde8
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 6 deletions.
18 changes: 12 additions & 6 deletions core/runtime/jsruntime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1456,7 +1456,7 @@ impl JsRuntime {
// and only then check for any promise exceptions (`unhandledrejection`
// handlers are run in macrotasks callbacks so we need to let them run
// first).
self.do_js_event_loop_tick(cx)?;
let dispatched_ops = self.do_js_event_loop_tick(cx)?;
self.check_promise_rejections()?;

// Event loop middlewares
Expand Down Expand Up @@ -1525,6 +1525,8 @@ impl JsRuntime {
if pending_state.has_pending_background_tasks
|| pending_state.has_tick_scheduled
|| maybe_scheduling
// If ops were dispatched we may have progress on pending modules that we should re-check
|| (pending_state.has_pending_module_evaluation && dispatched_ops)
{
state.op_state.borrow().waker.wake();
}
Expand Down Expand Up @@ -1854,11 +1856,13 @@ impl JsRuntime {
}

// Polls pending ops and then runs `Deno.core.eventLoopTick` callback.
fn do_js_event_loop_tick(&mut self, cx: &mut Context) -> Result<(), Error> {
fn do_js_event_loop_tick(&mut self, cx: &mut Context) -> Result<bool, Error> {
// Handle responses for each realm.
let state = self.inner.state.clone();
let isolate = &mut self.inner.v8_isolate;
let realm_count = state.borrow().known_realms.len();
let mut dispatched_ops = false;

for realm_idx in 0..realm_count {
let realm = state.borrow().known_realms.get(realm_idx).unwrap().clone();
let context_state = realm.state();
Expand Down Expand Up @@ -1891,6 +1895,7 @@ impl JsRuntime {
.tracker
.track_async_completed(op_id);
context_state.unrefed_ops.remove(&promise_id);
dispatched_ops |= true;
args.push(v8::Integer::new(scope, promise_id).into());
args.push(match resp.to_v8(scope) {
Ok(v) => v,
Expand All @@ -1900,8 +1905,9 @@ impl JsRuntime {
});
}

let has_tick_scheduled =
v8::Boolean::new(scope, self.inner.state.borrow().has_tick_scheduled);
let has_tick_scheduled = self.inner.state.borrow().has_tick_scheduled;
dispatched_ops |= has_tick_scheduled;
let has_tick_scheduled = v8::Boolean::new(scope, has_tick_scheduled);
args.push(has_tick_scheduled.into());

let js_event_loop_tick_cb_handle =
Expand All @@ -1919,10 +1925,10 @@ impl JsRuntime {
}

if tc_scope.has_terminated() || tc_scope.is_execution_terminating() {
return Ok(());
return Ok(false);
}
}

Ok(())
Ok(dispatched_ops)
}
}
69 changes: 69 additions & 0 deletions core/runtime/tests/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,18 @@
use crate as deno_core;
use crate::extensions::Op;
use crate::extensions::OpDecl;
use crate::modules::StaticModuleLoader;
use crate::runtime::tests::setup;
use crate::runtime::tests::Mode;
use crate::*;
use anyhow::Error;
use deno_ops::op;
use log::debug;
use std::cell::RefCell;
use std::rc::Rc;
use std::sync::atomic::Ordering;
use std::time::Duration;
use url::Url;

#[tokio::test]
async fn test_async_opstate_borrow() {
Expand Down Expand Up @@ -496,3 +500,68 @@ fn test_dispatch_stack_zero_copy_bufs() {
.unwrap();
assert_eq!(dispatch_count.load(Ordering::Relaxed), 1);
}

/// Test that long-running ops do not block dynamic imports from loading.
// https://github.com/denoland/deno/issues/19903
// https://github.com/denoland/deno/issues/19455
#[tokio::test]
pub async fn test_top_level_await() {
#[op2(async)]
async fn op_sleep_forever() {
tokio::time::sleep(Duration::MAX).await
}

deno_core::extension!(testing, ops = [op_sleep_forever]);

let loader = StaticModuleLoader::new([
(
Url::parse("http://x/main.js").unwrap(),
ascii_str!(
r#"
const { op_sleep_forever } = Deno.core.ensureFastOps();
(async () => await op_sleep_forever())();
await import('./mod.js');
"#
),
),
(
Url::parse("http://x/mod.js").unwrap(),
ascii_str!(
r#"
const { op_void_async_deferred } = Deno.core.ensureFastOps();
await op_void_async_deferred();
"#
),
),
]);

let mut runtime = JsRuntime::new(RuntimeOptions {
module_loader: Some(Rc::new(loader)),
extensions: vec![testing::init_ops()],
..Default::default()
});

let id = runtime
.load_main_module(&Url::parse("http://x/main.js").unwrap(), None)
.await
.unwrap();
let mut rx = runtime.mod_evaluate(id);

let res = tokio::select! {
// Not using biased mode leads to non-determinism for relatively simple
// programs.
biased;

maybe_result = &mut rx => {
debug!("received module evaluate {:#?}", maybe_result);
maybe_result.expect("Module evaluation result not provided.")
}

event_loop_result = runtime.run_event_loop(false) => {
event_loop_result.unwrap();
let maybe_result = rx.await;
maybe_result.expect("Module evaluation result not provided.")
}
};
res.unwrap();
}

0 comments on commit 39edde8

Please sign in to comment.