diff --git a/src/jsc/bindings/JSBundlerPlugin.cpp b/src/jsc/bindings/JSBundlerPlugin.cpp index ec33783d466e..383f783e17a5 100644 --- a/src/jsc/bindings/JSBundlerPlugin.cpp +++ b/src/jsc/bindings/JSBundlerPlugin.cpp @@ -667,7 +667,7 @@ extern "C" void JSBundlerPlugin__drainDeferred(Bun::JSBundlerPlugin* pluginObjec { auto* globalObject = pluginObject->globalObject(); MarkedArgumentBuffer arguments; - pluginObject->plugin.deferredPromises.moveTo(pluginObject, arguments); + pluginObject->plugin.deferredPromises.drainTo(pluginObject, arguments); ASSERT(!arguments.hasOverflowed()); auto& vm = pluginObject->vm(); diff --git a/src/jsc/bindings/WriteBarrierList.h b/src/jsc/bindings/WriteBarrierList.h index 0db6c5811c56..84345ca268f6 100644 --- a/src/jsc/bindings/WriteBarrierList.h +++ b/src/jsc/bindings/WriteBarrierList.h @@ -41,15 +41,17 @@ class WriteBarrierList { return m_list.mutableSpan(); } - void moveTo(JSC::JSCell* owner, JSC::MarkedArgumentBuffer& arguments) + // Move every element into `arguments` and clear the backing vector in one + // linear pass under a single cellLock. + void drainTo(JSC::JSCell* owner, JSC::MarkedArgumentBuffer& arguments) { WTF::Locker locker { owner->cellLock() }; + arguments.ensureCapacity(arguments.size() + m_list.size()); for (JSC::WriteBarrier& value : m_list) { - if (auto* cell = value.get()) { + if (auto* cell = value.get()) arguments.append(cell); - value.clear(); - } } + m_list.clear(); } template @@ -66,18 +68,6 @@ class WriteBarrierList { return m_list.isEmpty(); } - T* takeFirst(JSC::JSCell* owner) - { - WTF::Locker locker { owner->cellLock() }; - if (m_list.isEmpty()) { - return nullptr; - } - - T* value = m_list.first().get(); - m_list.removeAt(0); - return value; - } - template bool removeFirstMatching(JSC::JSCell* owner, const MatchFunction& matches) { diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index 52e6d8df2f68..8b7426cc656d 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -1053,6 +1053,18 @@ void GlobalObject::promiseRejectionTracker(JSGlobalObject* obj, JSC::JSPromise* return unhandledPromise.get() == promise; }); if (removed) break; + // handleRejectedPromises() drains the list into a local buffer before + // running any handler. A handler may .catch() a later still-queued + // promise; that promise is no longer in m_aboutToBeNotifiedRejectedPromises + // but has not yet had 'unhandledRejection' fired, so it must not get + // 'rejectionHandled'. Check every in-flight tail (handlers can re-enter + // handleRejectedPromises(), so there may be more than one). + for (auto* inflight = globalObj->m_rejectedPromisesBeingProcessed; inflight; inflight = inflight->outer) { + for (size_t i = inflight->index, n = inflight->buffer->size(); i < n; ++i) { + if (inflight->buffer->at(i).asCell() == promise) + return; + } + } // The promise rejection has already been notified, now we need to queue it for the rejectionHandled event Bun__handleHandledPromise(globalObj, promise); break; @@ -3278,18 +3290,41 @@ extern "C" void Bun__handleRejectedPromise(Zig::GlobalObject* JSGlobalObject, JS void GlobalObject::handleRejectedPromises() { + if (m_aboutToBeNotifiedRejectedPromises.isEmpty()) [[likely]] + return; + JSC::VM& virtual_machine = vm(); auto scope = DECLARE_TOP_EXCEPTION_SCOPE(virtual_machine); - while (auto* promise = m_aboutToBeNotifiedRejectedPromises.takeFirst(this)) { - if (promise->isHandled()) - continue; - - Bun__handleRejectedPromise(this, promise); - if (auto ex = scope.exception()) { - (void)scope.tryClearException(); - this->reportUncaughtExceptionAtEventLoop(this, ex); + do { + // Move the whole list out under one cellLock, then iterate linearly — + // the same pattern JSC's VM::didExhaustMicrotaskQueue and WebCore's + // RejectedPromiseTracker use. + JSC::MarkedArgumentBuffer promises; + m_aboutToBeNotifiedRejectedPromises.drainTo(this, promises); + RELEASE_ASSERT(!promises.hasOverflowed()); + // Expose the not-yet-processed tail so promiseRejectionTracker(Handle) + // can tell "still pending" apart from "already notified". Linked as a + // stack so a re-entrant handleRejectedPromises() (a handler that ticks + // the event loop) restores the outer frame instead of nulling it. + InFlightRejections inflight { &promises, 0, m_rejectedPromisesBeingProcessed }; + WTF::SetForScope inflightScope(m_rejectedPromisesBeingProcessed, &inflight); + for (size_t i = 0, size = promises.size(); i < size; ++i) { + auto* promise = static_cast(promises.at(i).asCell()); + if (promise->isHandled()) + continue; + inflight.index = i + 1; + + Bun__handleRejectedPromise(this, promise); + if (auto ex = scope.exception()) { + if (virtual_machine.isTerminationException(ex)) [[unlikely]] + return; + (void)scope.tryClearException(); + this->reportUncaughtExceptionAtEventLoop(this, ex); + } } - } + // An unhandledRejection handler may itself reject a promise; loop + // until the list stays empty. + } while (!m_aboutToBeNotifiedRejectedPromises.isEmpty()); } DEFINE_VISIT_CHILDREN(GlobalObject); diff --git a/src/jsc/bindings/ZigGlobalObject.h b/src/jsc/bindings/ZigGlobalObject.h index eae5d1cc5a6f..9bba7344771e 100644 --- a/src/jsc/bindings/ZigGlobalObject.h +++ b/src/jsc/bindings/ZigGlobalObject.h @@ -786,6 +786,21 @@ class GlobalObject : public Bun::GlobalScope { WebCore::SubtleCrypto* m_subtleCrypto = nullptr; Bun::WriteBarrierList m_aboutToBeNotifiedRejectedPromises; + +public: + // While handleRejectedPromises() is iterating its drained snapshot, this + // points at the not-yet-processed tail so promiseRejectionTracker(Handle) + // can suppress a spurious 'rejectionHandled' for a promise whose + // 'unhandledRejection' has not fired yet. Linked through `outer` to handle + // re-entrant handleRejectedPromises() calls. + struct InFlightRejections { + JSC::MarkedArgumentBuffer* buffer; + size_t index; + InFlightRejections* outer; + }; + +private: + InFlightRejections* m_rejectedPromisesBeingProcessed { nullptr }; }; class EvalGlobalObject : public GlobalObject { diff --git a/test/js/node/process/process.test.js b/test/js/node/process/process.test.js index 9d261c3861a0..00c694455113 100644 --- a/test/js/node/process/process.test.js +++ b/test/js/node/process/process.test.js @@ -806,6 +806,62 @@ describe.concurrent(() => { expect(await proc.exited).toBe(42); }); + it("delivers many unhandledRejections in order, including ones queued from the handler", async () => { + // Pins the observable behaviour: order is preserved, late .catch() + // suppresses delivery, and a rejection raised from inside the handler is + // also delivered. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const N = 1000; + const seen = []; + let nestedSeen = false; + process.on("unhandledRejection", reason => { + if (reason === "nested") { nestedSeen = true; return; } + seen.push(reason); + if (reason === 0) Promise.reject("nested"); + }); + for (let i = 0; i < N; i++) Promise.reject(i); + // This one is handled before the checkpoint runs — must NOT be delivered. + Promise.reject("handled").catch(() => {}); + await new Promise(r => setImmediate(r)); + await new Promise(r => setImmediate(r)); + if (seen.length !== N) throw new Error("count " + seen.length); + for (let i = 0; i < N; i++) if (seen[i] !== i) throw new Error("order at " + i + " got " + seen[i]); + if (seen.includes("handled")) throw new Error("handled promise was delivered"); + if (!nestedSeen) throw new Error("rejection from inside handler was dropped"); + + // A handler that .catch()es a *later* still-pending rejection must + // suppress both 'unhandledRejection' AND 'rejectionHandled' for it. + let spuriousRejectionHandled = 0; + let lateUnhandled = false; + process.on("rejectionHandled", () => spuriousRejectionHandled++); + let pLate; + process.removeAllListeners("unhandledRejection"); + process.on("unhandledRejection", reason => { + if (reason === "early") pLate.catch(() => {}); + if (reason === "late") lateUnhandled = true; + }); + Promise.reject("early"); + pLate = Promise.reject("late"); + await new Promise(r => setImmediate(r)); + await new Promise(r => setImmediate(r)); + if (lateUnhandled) throw new Error("late promise got unhandledRejection"); + if (spuriousRejectionHandled !== 0) + throw new Error("spurious rejectionHandled fired " + spuriousRejectionHandled + "x"); + console.log("ok"); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ stdout: "ok", stderr: "", exitCode: 0 }); + }); + it("aborts when the uncaughtException handler throws", async () => { const proc = Bun.spawn([bunExe(), join(import.meta.dir, "process-onUncaughtExceptionAbort.js")], { stderr: "pipe",