fix(core): prevent shared-buffer timer expiry race from losing timers (#35312)
This restores the timer expiry handoff changed in #32844 from the shared
`Float64Array` buffer back to the older return-value protocol.
The shared-buffer protocol wrote the next timer expiry during
`__eventLoopTick(...)`, then continued resolving completed async ops.
Promise hooks or op-resolution side effects could synchronously arm or
mutate timers after that snapshot was written. Rust then read the stale
snapshot after the JS call returned, allowing older timer state to
clear, delay, ref, or unref newer timer state.
With this change, Rust calls `Deno.core.__processTimers(now)`,
immediately commits the returned timer expiry, and only then resolves
completed async ops. This removes the stale shared-buffer overwrite
class.
## What changed
- Restored `Deno.core.__processTimers(now)`.
- Simplified `Deno.core.__eventLoopTick(...)` so it only resolves
completed async op results.
- Removed the shared `Float64Array` timer expiry handoff.
- Restored Rust-side scheduling from the `__processTimers(now)` return
value:
- positive value: next refed timer expiry
- negative value: next unrefed-only timer expiry
- `0`: no timers remain
- Added regression coverage for stale timer expiry states caused by
promise hooks during op resolution.
- Added coverage for timers armed while `processTimers()` drains
microtasks via `runNextTicks()`.
## Why return to the return-value protocol?
The shared-buffer design made the JS -> Rust handoff depend on a
snapshot that could become stale before Rust consumed it. Covering all
timer transitions correctly (`setTimeout`, `clearTimeout`, `refTimer`,
and `unrefTimer`) would require the shared state to represent both the
next deadline and ref/unref liveness without being overwritten by older
snapshots. That is hard to make robust when JS can synchronously mutate
timers while async op promises are being resolved in the same event-loop
turn.
The return-value protocol restores a simpler ordering guarantee: JS
processes the currently expired timers, returns the current next expiry,
and Rust commits that value immediately before resolving completed async
ops. That keeps JS and Rust timer state synchronized at the handoff
boundary.
The performance tradeoff also looks acceptable. The extra Rust -> JS
call can occur only in turns where both a native user timer and
completed async ops are ready, and the overlap-focused benchmarks below
did not show a stable, perceivable overhead from using the return-value
protocol.
## What kind of code could trigger this?
### Reduced repro
This uses `Deno.core.setPromiseHooks(...)` only to make the race
deterministic: it lets the repro run JS synchronously while an async op
promise is being resolved.
```js
const core = Deno[Deno.internal]?.core ?? Deno.core;
let armHook = false;
let scheduled = false;
// Run synchronously when a promise is resolved. The hook is initially inert;
// the first timer below enables it immediately before the file op resolves.
core.setPromiseHooks(null, null, null, () => {
if (!armHook || scheduled) return;
scheduled = true;
console.log("SCHEDULED");
// This is the timer that the old shared-buffer protocol could lose.
setTimeout(() => {
console.log("FIRED");
}, 1);
});
// This expired timer is processed first. In the broken case, it can publish
// a stale `0` snapshot via the shared Float64Array: "no next timer is armed".
setTimeout(() => {
console.log("EXPIRED");
armHook = true;
}, 0);
// Arrange for an async op promise to resolve in the same event-loop turn.
Deno.stat(import.meta.filename).then(() => {
console.log("OP_THEN");
});
// Let the zero-delay timer and file op become ready before yielding.
const start = performance.now();
while (performance.now() - start < 25) {}
```
On Deno 2.8.3, this locally reproduced the stale-`0` failure shape. In
runs where the hook armed the short timer, the broken output was:
```text
EXPIRED
SCHEDULED
OP_THEN
# FIRED is missing
```
In a 50-run local sample, this `SCHEDULED`-without-`FIRED` output
occurred 34 times. The expired timer publishes `timerExpiry[0] = 0`
before `Deno.stat(...)` is resolved. The promise hook then schedules
`setTimeout(..., 1)`, but Rust applies the older `0` snapshot after
returning from JS and clears the native timer. The newly scheduled timer
is left without a native wakeup.
With this change, the expected output is:
```text
EXPIRED
SCHEDULED
OP_THEN
FIRED
```
### Real-world scenario
A production-shaped scenario was a Deno Deploy build timeout while
running concurrent Slidev/Vite/esbuild builds. Several child processes
printed successful build output, but the parent sometimes stayed stuck
waiting for child status until the build timed out. The core of the
relevant application pattern is:
```js
const child = new Deno.Command(Deno.execPath(), {
cwd: path,
args: ["run", "build", "--base", base, "--out", outDir],
stdout: "piped",
stderr: "piped",
}).spawn();
console.log("waiting for child status");
const status = await child.status;
console.log(`child status resolved: ${status.code}`);
```
This is not using `setPromiseHooks` directly. It is ordinary
child-process orchestration with async op completions under concurrency.
Hook-like synchronous side effects may come from runtime internals, Node
compatibility, async context, observability, APM, or a test/build
harness. If such code arms or mutates a timer while Deno is resolving an
async op in the same turn as an expired timer, the old shared-buffer
protocol could apply an older timer snapshot afterwards and lose or
delay the newer timer state.
## Why this fixes the race
Old shared-buffer ordering:
```mermaid
sequenceDiagram
participant Rust
participant JS
participant Shared as shared Float64Array
Rust->>JS: __eventLoopTick(timerNow, completed op results...)
JS->>JS: process expired timers
JS->>Shared: write next-expiry snapshot
Note over Shared: This snapshot is only current until JS mutates timers again.
JS->>JS: resolve completed async ops
JS->>JS: promise hooks / op-resolution side effects arm or mutate timers
Note over JS,Shared: Any timer armed or changed here can make<br/> the shared snapshot stale.
JS-->>Rust: return
Rust->>Shared: read older next-expiry snapshot
Rust->>Rust: schedule/ref/unref from stale state
```
Restored return-value ordering:
```mermaid
sequenceDiagram
participant Rust
participant JS
Rust->>JS: __processTimers(now)
JS->>JS: process expired timers
JS-->>Rust: return current next expiry
Rust->>Rust: immediately schedule/ref/unref from return value
Rust->>JS: __eventLoopTick(completed op results...)
JS->>JS: resolve completed async ops
JS->>JS: promise hooks / op-resolution side effects arm or mutate timers
JS-->>Rust: return
```
If op resolution or promise hooks arm new timers, those updates happen
after the previous expiry has already been committed, so they cannot be
overwritten by a stale shared-buffer snapshot.
## Performance
The return protocol can add an extra Rust -> JS call only when both are
true in the same event-loop turn:
- the native user timer is ready
- completed async ops are ready
I benchmarked overlap-focused workloads that intentionally exercise that
shape.
Benchmark environment:
- OS: Linux 6.18.24, x86_64
- CPU: Ryzen 9 7940HS
- Shared-buffer baseline: 039bcdc6d before the local return-protocol
edits
- Return-protocol candidate: 039bcdc6d plus the local return-protocol
edits
<details>
<summary>Benchmark workload details (click to expand)</summary>
Each benchmark sample ran in a fresh Deno process. The full suite used 3
warmups and 11 measured runs.
`mixed_barrier_batch`
- 200 rounds.
- Each round schedules 100 zero-delay timers and 100 `Deno.stat(...)`
async ops.
- Total per sample: 20,000 timers and 20,000 async ops.
- Purpose: maximize same-turn timer/op overlap.
`timeout_guarded_stat`
- Two variants per sample:
- `long_guard`: 10,000 `Deno.stat(...)` calls guarded by a 1000 ms
timeout.
- `short_guard`: 10,000 `Deno.stat(...)` calls guarded by a 1 ms
timeout.
- Total per sample: 20,000 async ops and 20,000 timers created.
- Purpose: model production-shaped timeout guards around async I/O.
- The table below uses a focused rerun for this row: 5 warmups and 15
measured runs. The original full-suite run showed a median slowdown that
did not reproduce in the focused rerun, with similar high run-to-run
spread in both candidates.
`ready_op_plus_due_timer`
- 500 rounds.
- Each round schedules 20 zero-delay timers and 20 `Deno.stat(...)`
async ops.
- Total per sample: 10,000 timers and 10,000 async ops.
- Purpose: use more rounds with smaller batches to expose per-turn
overhead.
`timer_op_chain`
- 5,000 iterations.
- Each iteration awaits a zero-delay timer, then awaits
`Deno.stat(...)`.
- Purpose: stress repeated timer/async-op phase transitions.
`mixed_high_concurrency_tail`
- 10,000 tasks.
- Concurrency: 200.
- Each task awaits both a zero-delay timer and `Deno.stat(...)`.
- Purpose: measure throughput and per-task p50/p95/p99 tail latency
under mixed concurrency.
Every measured sample completed the expected timer and async-op counts.
</details>
| benchmark | shared median | return median | delta | shared p95 |
return p95 | p95 delta |
| --- | ---: | ---: | ---: | ---: | ---: | ---: |
| `mixed_barrier_batch` | 621.922 ms | 605.315 ms | -2.67% | 647.809 ms
| 645.313 ms | -0.39% |
| `timeout_guarded_stat` | 631.525 ms | 610.266 ms | -3.37% | 669.597 ms
| 682.193 ms | +1.88% |
| `ready_op_plus_due_timer` | 1251.606 ms | 1256.044 ms | +0.35% |
1259.849 ms | 1272.934 ms | +1.04% |
| `timer_op_chain` | 11104.208 ms | 11101.130 ms | -0.03% | 11161.269 ms
| 11171.279 ms | +0.09% |
| `mixed_high_concurrency_tail` | 353.346 ms | 352.605 ms | -0.21% |
374.607 ms | 363.297 ms | -3.02% |
These overlap-focused benchmarks do not show a stable, perceivable
overhead from the return-value protocol in the mixed timer/async-op
shape where the extra JS call can occur.
deno
49c38256 - fix(core): prevent shared-buffer timer expiry race from losing timers (#35312)
