Skip to content

Commit

Permalink
Tidy up event loop handling code (#697)
Browse files Browse the repository at this point in the history
* Tidy up event loop handling code

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)

* Clippy fixes

* Assert fuel conditionally

When the `experimental_event_loop` feature is enabled, a bit more work
is done when executing JS code, therefore there's more fuel usage
reported in some tests.

* Create `cli-features.yml`

* Add right naming

* Fix `runs-on` key

* Typo and fixes
  • Loading branch information
saulecabrera authored Jul 8, 2024
1 parent 0bfd2bf commit f8dea1b
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 32 deletions.
23 changes: 23 additions & 0 deletions .github/workflows/cli-features.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Tests extra CLI features and their dependency with core features.
name: Test CLI Features
on:
push:
branches:
- main
pull_request:

jobs:
cli:
name: Test CLI Features
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- uses: ./.github/actions/ci-shared-setup
with:
os: linux

- name: Test `experimental_event_loop`
run: |
cargo build --package=javy-core --target=wasm32-wasi --release --features=experimental_event_loop
cargo test --package=javy-cli --features=experimental_event_loop --release -- --nocapture
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);
66 changes: 34 additions & 32 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,47 @@ 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()?
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 f8dea1b

Please sign in to comment.