JavaScriptEventLoop: release the isSpinning latch before running jobs

[mansbernhardt]

JobQueue.runAllJobs() clears queueState.isSpinning only as its final statement:

func runAllJobs() {
    assert(queueState.isSpinning)

    while let job = self.claimNextFromQueue() {
        job.runSynchronously(on: self.asUnownedSerialExecutor())
    }

    queueState.isSpinning = false   // ← not reached if a job unwinds
}

If runSynchronously unwinds, that line never runs and isSpinning stays true. insertJobQueue schedules a drain only when !isSpinning:

if !queueState.isSpinning {
    self.queueState.isSpinning = true
    JavaScriptEventLoop.shared.queueMicrotask { self.runAllJobs() }
}

So after a single unwound job the queue is never drained again — every later enqueue appends to a queue nothing will run, for the lifetime of the process.

Why it is hard to spot

The failure is silent and total for async work while synchronous entry points keep behaving normally, so it presents as "async stopped" rather than as a crash. In a browser it is worse than that: queueTask is implemented as promise.then { job() }, so the escaping error rejects a discarded promise — it surfaces as an unhandled rejection, never as window.onerror. A page in this state answers every synchronous export perfectly and looks healthy.

How we hit it

We ship a Swift/wasm app on JavaScriptKit 0.57.0. A trap inside a job (in our case an AsyncAlgorithms merge precondition, but the origin doesn't matter) left every Swift Task permanently dead while the module kept answering synchronous calls in ~4 ms. It cost us about nine days to attribute, because every measurement of the module said it was fine.

We confirmed the mechanism by injection with a control arm: one self-reverting throw from a host import → the next intent call times out at 21,986 ms; without it → 12 ms. We can share that harness if useful.

The fix

Release the latch before the loop, so isSpinning means "a drain microtask is pending" rather than "a drain is in progress". An unwinding job then leaves the queue undrained but unlatched, and the next insertJobQueue schedules a fresh drain — including for jobs already queued behind it.

An earlier revision of this PR used defer. That was wrong: defer runs for a Swift throw, not for a wasm trap or a JS exception crossing back into wasm — the only two paths that strand the latch. Thanks to @kateinoigakukun for catching it.

Measured on a standalone repro (Node, no browser): enqueue one job that unwinds, then ask whether a Task enqueued afterwards ever runs. 5/5 runs per cell.

JobQueue	control	JS exception	Swift trap
0.57.0 as-is	alive	dead	dead
+ defer	alive	dead	dead
release before the loop	alive	alive	alive

Costs 2 extra drain microtasks per 20,000 await hops (work enqueued during a drain is consumed by the running loop without returning, so drains stay rare); no measurable wall-clock change. Existing suite unchanged: 194/194 XCTest + 13/13 swift-testing.

On a regression test

There isn't one, deliberately. Fixed and unfixed differ only under an unwind, and an unwind is fatal to any in-process host: an XCTest case that throws through runAllJobs takes the Node process down and ~9 sibling tests with it, identically with and without the fix. Asserting the invariant directly doesn't work either — "isSpinning is false while a job runs" is false even when fixed, since any job that enqueues legitimately re-arms it.

A real test needs an out-of-process harness: build a fixture, run it under Node with a host that tolerates the escaping exception, assert a later Task still runs. That's a new shape for this repo (Runtime/test is JS-only against a stubbed instance; Examples/* are built but never executed), so I didn't want to invent it unasked. Happy to add one wherever you'd want it.

The same trailing-assignment pattern is in the unmerged generalize-jobqueue branch (PriorityQueue.swift), so it would ship again unless fixed there too.

[kateinoigakukun]

I think for such scenarios where an unwind happens outside of Swift's throws mechanism, the defer block won't be executed, so I don't think this change solves the issue?

[mansbernhardt]

You're right. Measured it:

JobQueue	control	JS exception	Swift trap
0.57.0	alive	dead	dead
+ defer	alive	dead	dead
release latch before the loop	alive	alive	alive

Standalone repro under Node, 5/5 runs per cell. And on wasm32 a defer runs for a Swift throw but not for a precondition failure — so it was simply the wrong instrument.

Pushed the early-release version instead. isSpinning now means "a drain microtask is pending" rather than "a drain is running", so an unwind leaves the queue unlatched and the next insertJobQueue reschedules. Recovers jobs queued behind the unwinding one too. Costs 2 extra microtasks per 20k await hops; suite unchanged at 194/194 + 13/13.

No regression test, deliberately. Fixed and unfixed differ only under an unwind, and an unwind kills the test host — an XCTest case that throws through runAllJobs takes the Node process down and ~9 sibling tests with it, identically with and without the fix. Asserting the invariant directly fails too: "isSpinning is false while a job runs" is false even when fixed, since any enqueuing job legitimately re-arms it. A real test needs an out-of-process harness — Runtime/test is JS-only against a stubbed instance, Examples/* are built but never run. Happy to add one wherever you'd want it; the repro is ~50 lines of Swift plus a ~40-line Node driver.

[kateinoigakukun]

Hmm, your new approach changes the semantics of the scheduler. With that change, a job enqueued during a spin is no longer executed within the same spin, which is not acceptable for very latency-aware applications.

Also first of all, I'd say any Wasm instance that is unwound by a JS exception or has already crashed is dead, and it's program state is unhealthy (e.g. stack pointer is not updated by each unwound stack epilogue), so apps should not re-enter such instances. So I'd suggest wrapping all instance.exports functions and monitoring exceptions thrown by them, and if any of them throws, then you can reject further export calls. This can be implemented at the application level without JavaScriptKit change.

[mansbernhardt]

Fair enough on re-entering a dead instance — that's the stronger position and I'll take it. I've implemented your suggestion: wrapping instance.exports and refusing every later call once one unwinds. It catches both a JS exception and a Swift trap in a minimal repro, and our app's standing trap-regression arm is unaffected (40 hops, clean). It's better than what we had, since guarding imports can't see a Swift trap. One wrinkle for anyone else doing this: instance.exports properties are non-configurable, so a Proxy get trap violates the invariants — you have to rebuild exports as a plain object.

One correction for the record: the early release doesn't defer same-spin work. The drain loop is unchanged, so a job enqueued during a spin is still claimed by that same loop — it only schedules one redundant microtask that finds an empty queue. Measured with a spin counter, negative control being a job enqueued from a later JS turn:

arm	during the spin	from a later turn
0.57.0	same spin	different spin
early release	same spin	different spin

Cost was 2 extra microtasks per 20,000 await hops.

But if the instance shouldn't be re-entered anyway, the latch's permanence is moot — closing. The one thing I'd still flag is that a wedged instance answers every synchronous export perfectly, so nothing surfaces the fault; that's what sent me to JobQueue. Worth a diagnostic somewhere, but not this change. Thanks for the review.

[kateinoigakukun]

Thank you for raising this!