Repository navigation
Disallow events in FutexEmulation, but order Wait calls - #170
Merged
Merged
Conversation
Port of the Chromium V8 fork's "[RUN-2378] Disallow events in FutexEmulation, but order Wait calls" (replayio/chromium-v8 nodejs#199), as the file stands there today: the futex emulation's own synchronization (its global mutex, the per-node condition variables and the wait list) runs with events disallowed in NotifyWake, AtomicsWaitWakeHandle::Wake, WaitSync, WaitAsync, Wake, ResolveAsyncWaiterPromises, HandleAsyncWaiterTimeout, IsolateDeinit and the *ForTesting counters, so none of it is recorded, and the public Wait entry point instead asserts on entry and exit through the new recordreplay::AutoAssertMaybeEventsDisallowed scope, which records the order of Wait calls. Until now the wait was recorded through its condition variable, and a recording of a worker blocking in Atomics.wait while the main thread notifies it could not be replayed (assert mismatch on the worker thread). Differences from the Chromium change: - The scopes use node's existing v8::replayio::AutoDisallowEvents (the Chromium file was renamed to it in nodejs#226). - AutoAssertMaybeEventsDisallowed is declared next to AssertMaybeEventsDisallowed (node has no AutoOrderedLock to follow), formats with vsnprintf and asserts through the existing AssertMaybeEventsDisallowed, which does the same checks. - Tasks are not posted while events are disallowed: node's platform posts through a task queue mutex and uv_async_send, which are the events that order the receiving thread after the post, and asserts in PostTask. Wake collects the tasks resolving other isolates' async waiter promises and posts them after releasing the lock and ending the scope; with the posting inside the scope, a worker's Atomics.waitAsync woken by the main thread replayed with an empty task queue and diverged. WaitAsync posts its timeout task the same way, which otherwise only leaves warnings in the recording since it is posted to the waiter's own isolate. - A post from another thread could already race the waiter isolate's PerIsolatePlatformData::Shutdown, since node unregisters an isolate from the platform before disposing it and nothing synchronized the flush handle; posting after releasing the futex lock widens that window, so the platform now guards the handle with an ordered mutex and discards a task posted during or after Shutdown. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Andarist
force-pushed
the
andarist/futex-emulation-disallow-events
branch
from
October 6, 2026 09:09
ee34fb2 to
df13e52
Compare
Andarist
added a commit
that referenced
this pull request
Oct 7, 2026
A worker blocked in Atomics.wait() and terminated while recording never stopped, so the process never exited. The stop watchdog (#173) invalidated the recording after 5s and terminated the worker's execution, which woke the futex wait, but FutexEmulation::WaitSync handles interrupts with events disallowed (#170), and StackGuard::HandleInterrupts ignores interrupts while events are disallowed (#173, ported from replayio/chromium-v8#30). The termination stayed pending and the waiter went back to waiting on the futex emulation's condition variable, with the parent idle in its event loop, waiting for the worker's exit. WaitSync now handles a termination that HandleInterrupts left pending. Only another thread can terminate a thread blocked there, which already invalidates the recording (StackGuard::RequestInterrupt), so this doesn't change what a usable recording or its replay does. Other interrupts are still left for the next stack check, and without recording HandleInterrupts has already handled the termination, so the check is a no-op. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Andarist
added a commit
that referenced
this pull request
Oct 7, 2026
* Terminate workers blocked in Atomics.wait() when recording A worker blocked in Atomics.wait() and terminated while recording never stopped, so the process never exited. The stop watchdog (#173) invalidated the recording after 5s and terminated the worker's execution, which woke the futex wait, but FutexEmulation::WaitSync handles interrupts with events disallowed (#170), and StackGuard::HandleInterrupts ignores interrupts while events are disallowed (#173, ported from replayio/chromium-v8#30). The termination stayed pending and the waiter went back to waiting on the futex emulation's condition variable, with the parent idle in its event loop, waiting for the worker's exit. WaitSync now handles a termination that HandleInterrupts left pending. Only another thread can terminate a thread blocked there, which already invalidates the recording (StackGuard::RequestInterrupt), so this doesn't change what a usable recording or its replay does. Other interrupts are still left for the next stack check, and without recording HandleInterrupts has already handled the termination, so the check is a no-op. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Stop workers right away once the recording is finished When recording or replaying, Worker::Exit called from another thread posts the stop to the worker's event loop and starts the stop watchdog (#173), so that the worker stops at a point the replay can reproduce. When the parent exits, through process.exit() or the end of its event loop, the recording is already finished (RecordReplayFinishRecording runs before stop_sub_worker_contexts), so there is nothing left to reproduce: a worker that never returned to its event loop, e.g. one blocked in Atomics.wait(), held up the exit for the watchdog's 5s for no benefit. Worker::Exit now stops the worker right away once the recording is finished, as without recording. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Port of the Chromium V8 fork's "[RUN-2378] Disallow events in FutexEmulation, but order Wait calls" (replayio/chromium-v8 nodejs#199), as the file stands there today. The futex emulation's own synchronization (its global mutex, the per-node condition variables and the wait list) now runs with events disallowed in NotifyWake, AtomicsWaitWakeHandle::Wake, WaitSync, WaitAsync, Wake, ResolveAsyncWaiterPromises, HandleAsyncWaiterTimeout, IsolateDeinit and the *ForTesting counters, so none of it is recorded. The public Wait entry point instead asserts on entry and exit through the new
recordreplay::AutoAssertMaybeEventsDisallowedscope, which records the order of Wait calls.Until now the wait was recorded through its condition variable. A recording of a worker blocking in Atomics.wait while the main thread notifies it could not be replayed: every replay crashed with an assert mismatch on the worker thread.
Differences from the Chromium change
v8::replayio::AutoDisallowEvents(Chromium renamed to it in [TT-819] Fix Asserts in RejectPromise chromium-v8#226).AutoAssertMaybeEventsDisallowedis declared next toAssertMaybeEventsDisallowed, formats with vsnprintf and asserts through that existing function, which does the same checks.PerIsolatePlatformData::Shutdown: node unregisters an isolate from the platform before disposing it, and nothing synchronized the flush handle. Posting after releasing the futex lock widens that window, so the platform now guards the handle with an ordered mutex (it nests the task queue's and the async handle's ordered locks) and discards a task posted during or after Shutdown.Testing
Backend case
test/node-recording/tests/worker-threads/atomics-wait-notify(branchclaude/node-recording-gc-tests): a worker blocks in Atomics.wait with and without a timeout on cells the main thread notifies after a timer, other waits time out or find a changed value, and the same outcomes go through Atomics.waitAsync in the worker and in the main thread; the replay must report the same outcomes and woken counts.tests/worker-threadspasses;tests/gchas the same 11 failures as master (they need the old-space-GC stack).Not covered by the test: AtomicsWaitWakeHandle::Wake and the *ForTesting paths.