Skip to content

Commit

Permalink
Tidy up event loop handling code
Browse files Browse the repository at this point in the history
This commit refactors the event loop handling code, mainly by extracting
the common pieces into a reusable function.

Additionally, this commit ensures that the result of `Promise.finish` is
corectly handled, which fixes execution of code with top level awaits.

Finally, this commit also ensures that we test the
`experimental_event_loop` in CI for the CLI and core crates (follow-up
to #238)
  • Loading branch information
saulecabrera committed Jul 8, 2024
1 parent d4a6e14 commit e902203
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 35 deletions.
9 changes: 8 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,14 @@ jobs:
path: target/wasm32-wasi/release/

- name: Test CLI
run: CARGO_PROFILE_RELEASE_LTO=off cargo test --package=javy-cli --release -- --nocapture
env:
CARGO_PROFILE_RELEASE_LTO: off
run: |
cargo build --package=javy-core --target=wasm32-wasi --release
cargo test --package=javy-cli --release -- --nocapture
cargo build --package=javy-core --target=wasm32-wasi --release --features=experimental_event_loop
cargo test --package=javy-cli --features=experimental_event_loop --release -- --nocapture
- name: Check benchmarks
run: CARGO_PROFILE_RELEASE_LTO=off cargo check --package=javy-cli --release --benches
Expand Down
14 changes: 14 additions & 0 deletions crates/cli/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,20 @@ fn test_promises() -> Result<()> {
Ok(())
}

#[cfg(feature = "experimental_event_loop")]
#[test]
fn test_promise_top_level_await() -> Result<()> {
let mut runner = Builder::default()
.bin(BIN)
.root(sample_scripts())
.input("top-level-await.js")
.build()?;
let (out, _, _) = run(&mut runner, &[]);

assert_eq!("bar", String::from_utf8(out)?);
Ok(())
}

#[test]
fn test_exported_functions() -> Result<()> {
let mut runner = Builder::default()
Expand Down
6 changes: 6 additions & 0 deletions crates/cli/tests/sample-scripts/top-level-await.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
async function foo() {
return Promise.resolve("bar");
}

const output = new TextEncoder().encode(await foo());
Javy.IO.writeSync(1, output);
72 changes: 38 additions & 34 deletions crates/core/src/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::process;
use anyhow::{anyhow, bail, Error, Result};
use javy::{
from_js_error,
quickjs::{context::EvalOptions, Module, Value},
quickjs::{context::EvalOptions, Ctx, Error as JSError, Module, Value},
to_js_error, Runtime,
};

Expand All @@ -24,21 +24,10 @@ pub fn run_bytecode(runtime: &Runtime, bytecode: &[u8]) {
let module = unsafe { Module::load(this.clone(), bytecode)? };
let (_, promise) = module.eval()?;

if cfg!(feature = "experimental_event_loop") {
// If the experimental event loop is enabled, trigger it.
promise.finish::<Value>().map(|_| ())
} else {
// Else we simply expect the promise to resolve immediately.
match promise.result() {
None => Err(to_js_error(this, anyhow!(EVENT_LOOP_ERR))),
Some(r) => r,
}
}
handle_maybe_promise(this.clone(), promise.into())
})
.map_err(|e| runtime.context().with(|cx| from_js_error(cx.clone(), e)))
// Prefer calling `process_event_loop` *outside* of the `with` callback,
// to avoid errors regarding multiple mutable borrows.
.and_then(|_| process_event_loop(runtime))
.and_then(|_: ()| ensure_pending_jobs(runtime))
.unwrap_or_else(handle_error)
}

Expand All @@ -63,34 +52,49 @@ pub fn invoke_function(runtime: &Runtime, fn_module: &str, fn_name: &str) {
opts.global = false;
let value = this.eval_with_options::<Value<'_>, _>(js, opts)?;

if let Some(promise) = value.as_promise() {
if cfg!(feature = "experimental_event_loop") {
// If the experimental event loop is enabled, trigger it.
promise.finish::<Value>().map(|_| ())
handle_maybe_promise(this.clone(), value)
})
.map_err(|e| runtime.context().with(|cx| from_js_error(cx.clone(), e)))
.and_then(|_: ()| ensure_pending_jobs(runtime))
.unwrap_or_else(handle_error)
}

/// Handles the promise returned by evaluating the JS bytecode.
fn handle_maybe_promise(this: Ctx, value: Value) -> javy::quickjs::Result<()> {
match value.as_promise() {
Some(promise) => {
if cfg!(feature = "experimental_event_loop") {
// If the experimental event loop is enabled, trigger it.
let resolved = promise.finish::<Value>();
// `Promise::finish` returns Err(Wouldblock) when the all
// pending jobs have been handled.
if let Err(JSError::WouldBlock) = resolved {
Ok(())
} else {
// Else we simply expect the promise to resolve immediately.
match promise.result() {
None => Err(to_js_error(this, anyhow!(EVENT_LOOP_ERR))),
Some(r) => r,
}
resolved.map(|_| ())
}
} else {
Ok(())
// Else we simply expect the promise to resolve immediately.
match promise.result() {
None => Err(to_js_error(this, anyhow!(EVENT_LOOP_ERR))),
Some(r) => r,
}
}
})
.map_err(|e| runtime.context().with(|cx| from_js_error(cx.clone(), e)))
.and_then(|_: ()| process_event_loop(runtime))
.unwrap_or_else(handle_error)
}
None => Ok(()),
}
}

fn process_event_loop(rt: &Runtime) -> Result<()> {
fn ensure_pending_jobs(rt: &Runtime) -> Result<()> {
if cfg!(feature = "experimental_event_loop") {
rt.resolve_pending_jobs()?
} else if rt.has_pending_jobs() {
bail!(EVENT_LOOP_ERR);
rt.resolve_pending_jobs()
} else {
if rt.has_pending_jobs() {
bail!(EVENT_LOOP_ERR);
} else {
Ok(())
}
}

Ok(())
}

fn handle_error(e: Error) {
Expand Down

0 comments on commit e902203

Please sign in to comment.