Skip to content

Avoid fully awaiting futures in async event loops#4332

Merged
jedel1043 merged 1 commit intomainfrom
fix-async-event-loops
Jul 12, 2025
Merged

Avoid fully awaiting futures in async event loops#4332
jedel1043 merged 1 commit intomainfrom
fix-async-event-loops

Conversation

@jedel1043
Copy link
Member

@jedel1043 jedel1043 commented Jul 12, 2025

This fixes a small bug where a single future that enqueues a promise job could block executing the promise job queue. For example:

fn delay(
    _this: &JsValue,
    args: &[JsValue],
    context: &RefCell<&mut Context>,
) -> impl Future<Output = JsResult<JsValue>> {
    let millis = args.get_or_undefined(0).to_u32(&mut context.borrow_mut());

    context
        .borrow_mut()
        .enqueue_job(Job::PromiseJob(PromiseJob::new(|_| {
            println!("ran a job!");
            Ok(JsValue::undefined())
        })));

    async move {
        let millis = millis?;
        let now = Instant::now();
        smol::Timer::after(Duration::from_millis(u64::from(millis))).await;
        let elapsed = now.elapsed().as_secs_f64();
        Ok(elapsed.into())
    }
}

In the previous implementation of the event loops, the queued job would only execute if the returned async method finished executing. With the new implementation, every loop only polls the pending futures once before draining the promise job queue again.

@jedel1043 jedel1043 requested a review from a team July 12, 2025 02:34
@jedel1043 jedel1043 added the A-Bug Something isn't working label Jul 12, 2025
@github-actions
Copy link

Test262 conformance changes

Test result main count PR count difference
Total 50,595 50,595 0
Passed 47,238 47,238 0
Ignored 1,964 1,964 0
Failed 1,393 1,393 0
Panics 0 0 0
Conformance 93.36% 93.36% 0.00%

@jedel1043 jedel1043 added this to the next-release milestone Jul 12, 2025
Copy link
Member

@HalidOdat HalidOdat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! :)

@jedel1043 jedel1043 added this pull request to the merge queue Jul 12, 2025
Merged via the queue into main with commit 48a8e2a Jul 12, 2025
15 checks passed
@jedel1043 jedel1043 deleted the fix-async-event-loops branch July 12, 2025 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants