Skip to content

Fix EventLoop async API: signal every concurrent WaitForEventAsync wa…#2427

Merged
lahma merged 1 commit into
sebastienros:mainfrom
Ellested:fix/eventloop-multi-waiter
May 14, 2026
Merged

Fix EventLoop async API: signal every concurrent WaitForEventAsync wa…#2427
lahma merged 1 commit into
sebastienros:mainfrom
Ellested:fix/eventloop-multi-waiter

Conversation

@Ellested

Copy link
Copy Markdown
Contributor

…iter

Repro: any caller pattern that issues two concurrent WaitForEventAsync calls against a single EventLoop drops the wake signal to the second caller.

Root cause: the loop tracked the wake TCS in a single volatile field (_eventAvailable). When a second caller arrived while the first was still pending, the new TCS was rejected by CompareExchange and the caller fell into the "Active waiter exists — events may already be pending" branch, returning Task.CompletedTask without registering any wake signal. The caller's outer loop would then re-enter WaitForEventAsync on the next IsEmpty=true check and immediately get Task.CompletedTask again — a CPU-burning spin until the queue happened to drain by some other path.

Fix: hold a list of TCS waiters under a Lock, append per call, and broadcast TrySetResult to every entry on Enqueue. The list is cleared on each broadcast so subsequent Enqueues don't re-signal stale entries; spurious wakes (e.g. self-signaling on the post-registration double-check) are harmless because the caller's outer loop re-checks its own state.

Added Jint.Tests/Runtime/AsyncTests.cs::EventLoopShouldSignalAllConcurrentWaiters which fails on the previous code (waiter1 is signaled, waiter2 stays pending after Enqueue) and passes with the fix.

The single-waiter cancellation path (CancellationTokenRegistration disposal via tcs.Task.ContinueWith) is preserved unchanged for the common case where only one waiter is registered.

…iter

Repro: any caller pattern that issues two concurrent WaitForEventAsync
calls against a single EventLoop drops the wake signal to the second
caller.

Root cause: the loop tracked the wake TCS in a single volatile field
(_eventAvailable). When a second caller arrived while the first was
still pending, the new TCS was rejected by CompareExchange and the
caller fell into the "Active waiter exists — events may already be
pending" branch, returning Task.CompletedTask without registering any
wake signal. The caller's outer loop would then re-enter
WaitForEventAsync on the next IsEmpty=true check and immediately
get Task.CompletedTask again — a CPU-burning spin until the queue
happened to drain by some other path.

Fix: hold a list of TCS waiters under a Lock, append per call, and
broadcast TrySetResult to every entry on Enqueue. The list is cleared
on each broadcast so subsequent Enqueues don't re-signal stale entries;
spurious wakes (e.g. self-signaling on the post-registration
double-check) are harmless because the caller's outer loop re-checks
its own state.

Added Jint.Tests/Runtime/AsyncTests.cs::EventLoopShouldSignalAllConcurrentWaiters
which fails on the previous code (waiter1 is signaled, waiter2 stays
pending after Enqueue) and passes with the fix.

The single-waiter cancellation path (CancellationTokenRegistration
disposal via tcs.Task.ContinueWith) is preserved unchanged for the
common case where only one waiter is registered.
@lahma
lahma force-pushed the fix/eventloop-multi-waiter branch from 9ed6540 to 0962d38 Compare May 14, 2026 08:14

@lahma lahma left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, thank you!

@lahma
lahma enabled auto-merge (squash) May 14, 2026 08:16
@lahma
lahma merged commit 96673d3 into sebastienros:main May 14, 2026
7 of 8 checks passed
lahma added a commit to lahma/jint that referenced this pull request May 14, 2026
Bumps the test262 suite to commit 673e9bac, which brings stage-4
spec updates for the immutable-arraybuffer proposal and a stricter
ArrayBuffer.prototype.sliceToImmutable algorithm. Twelve test
cases now pass with the following fixes:

- ArrayBufferPrototype.SliceToImmutable: rewrite to the new spec
  algorithm. first/final are still computed from the original len,
  but the detach check now precedes the bounds check and a
  RangeError is thrown when the buffer has shrunk below final
  during coercion.
- TypeConverter.ToNumber(string): guard against empty input after
  TrimEx. The pre-trim IsNullOrWhiteSpace check misses BOM and
  other characters that the spec-compliant TrimEx removes, which
  previously caused an IndexOutOfRangeException on the
  whitespace-padded empty string used by the new
  argument-coercion.js test.
- Uint8Array.prototype.setFromBase64 / setFromHex: assert the
  viewed buffer is not immutable before writing, per the new
  spec step.
- %TypedArray%.from / .of: assert the buffer of a custom
  constructor's result is not immutable before populating it
  (the "write" mutability intent in TypedArrayCreateFromConstructor).
- TypedArrayConstructor.IterableToList: pass the already-fetched
  iterator method through to GetIterator instead of re-reading
  Symbol.iterator on the source.

Also fixes a CI-host crash on PR sebastienros#2427: a FinalizationRegistry
cleanup callback can throw arbitrary exceptions (e.g. TimeoutException
when engine constraints trip), and they were escaping the GC
finalizer thread and aborting the entire test process. The
callback invocation is now wrapped in a swallow-all try/catch,
matching the spec's intent that these callbacks run in an
isolated Job.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lahma added a commit that referenced this pull request May 14, 2026
Bumps the test262 suite to commit 673e9bac, which brings stage-4
spec updates for the immutable-arraybuffer proposal and a stricter
ArrayBuffer.prototype.sliceToImmutable algorithm. Twelve test
cases now pass with the following fixes:

- ArrayBufferPrototype.SliceToImmutable: rewrite to the new spec
  algorithm. first/final are still computed from the original len,
  but the detach check now precedes the bounds check and a
  RangeError is thrown when the buffer has shrunk below final
  during coercion.
- TypeConverter.ToNumber(string): guard against empty input after
  TrimEx. The pre-trim IsNullOrWhiteSpace check misses BOM and
  other characters that the spec-compliant TrimEx removes, which
  previously caused an IndexOutOfRangeException on the
  whitespace-padded empty string used by the new
  argument-coercion.js test.
- Uint8Array.prototype.setFromBase64 / setFromHex: assert the
  viewed buffer is not immutable before writing, per the new
  spec step.
- %TypedArray%.from / .of: assert the buffer of a custom
  constructor's result is not immutable before populating it
  (the "write" mutability intent in TypedArrayCreateFromConstructor).
- TypedArrayConstructor.IterableToList: pass the already-fetched
  iterator method through to GetIterator instead of re-reading
  Symbol.iterator on the source.

Also fixes a CI-host crash on PR #2427: a FinalizationRegistry
cleanup callback can throw arbitrary exceptions (e.g. TimeoutException
when engine constraints trip), and they were escaping the GC
finalizer thread and aborting the entire test process. The
callback invocation is now wrapped in a swallow-all try/catch,
matching the spec's intent that these callbacks run in an
isolated Job.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants