diff --git a/src/js/builtins/ProcessObjectInternals.ts b/src/js/builtins/ProcessObjectInternals.ts index 37681a67f7c6..798873b536ed 100644 --- a/src/js/builtins/ProcessObjectInternals.ts +++ b/src/js/builtins/ProcessObjectInternals.ts @@ -387,6 +387,7 @@ export function initializeNextTickQueue( drainMicrotasks(); } while (!queue.isEmpty()); + $putInternalField(nextTickQueue, 0, 0); } $putInternalField(nextTickQueue, 0, 0); diff --git a/src/jsc/bindings/BunProcess.cpp b/src/jsc/bindings/BunProcess.cpp index 8b5b4c123b76..cc776b6292ab 100644 --- a/src/jsc/bindings/BunProcess.cpp +++ b/src/jsc/bindings/BunProcess.cpp @@ -3934,6 +3934,8 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionReportUncaughtException, (JSC::JSGlobalObject JSC_DEFINE_HOST_FUNCTION(jsFunctionDrainMicrotaskQueue, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) { + // Only caller is processTicksAndRejections; suppress the onEachMicrotaskTick hook while it drains. + WTF::SetForScope drainingGuard(defaultGlobalObject(globalObject)->m_isDrainingNextTickQueue, true); JSC::getVM(globalObject).drainMicrotasks(); return JSValue::encode(jsUndefined()); } diff --git a/src/jsc/bindings/JSNextTickQueue.cpp b/src/jsc/bindings/JSNextTickQueue.cpp index 71bba18387b8..22aeb129486c 100644 --- a/src/jsc/bindings/JSNextTickQueue.cpp +++ b/src/jsc/bindings/JSNextTickQueue.cpp @@ -77,20 +77,14 @@ bool JSNextTickQueue::isEmpty() void JSNextTickQueue::drain(JSC::VM& vm, JSC::JSGlobalObject* globalObject) { auto throwScope = DECLARE_THROW_SCOPE(vm); - bool mustResetContext = false; if (isEmpty()) { RETURN_IF_EXCEPTION(throwScope, ); vm.drainMicrotasks(); RETURN_IF_EXCEPTION(throwScope, ); - mustResetContext = true; } if (!isEmpty()) { RETURN_IF_EXCEPTION(throwScope, ); - if (mustResetContext) { - globalObject->m_asyncContextData.get()->putInternalField(vm, 0, jsUndefined()); - RETURN_IF_EXCEPTION(throwScope, ); - } auto* drainFn = internalField(2).get().getObject(); MarkedArgumentBuffer drainArgs; JSC::call(globalObject, drainFn, drainArgs, "Failed to drain next tick queue"_s); diff --git a/src/jsc/bindings/NodeAsyncHooks.cpp b/src/jsc/bindings/NodeAsyncHooks.cpp index 58beb067de3d..eb490a2c7bd7 100644 --- a/src/jsc/bindings/NodeAsyncHooks.cpp +++ b/src/jsc/bindings/NodeAsyncHooks.cpp @@ -17,9 +17,7 @@ using namespace JSC; JSC_DEFINE_HOST_FUNCTION(jsCleanupLater, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) { ASSERT(callFrame->argumentCount() == 0); - auto* global = uncheckedDowncast(globalObject); - global->asyncHooksNeedsCleanup = true; - global->resetOnEachMicrotaskTick(); + uncheckedDowncast(globalObject)->resetOnEachMicrotaskTick(); return JSC::JSValue::encode(JSC::jsUndefined()); } diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index 4590e7bce691..b70f53552830 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -389,23 +389,23 @@ extern "C" JSC::EncodedJSValue BunObject__createBunStdout(JSC::JSGlobalObject*); static void checkIfNextTickWasCalledDuringMicrotask(JSC::VM& vm) { auto* globalObject = defaultGlobalObject(); - if (auto queue = globalObject->m_nextTickQueue.get()) { - globalObject->resetOnEachMicrotaskTick(); - queue->drain(vm, globalObject); - } + // Guard only this hook, not drain(): GlobalObject::drainMicrotasks must still + // re-enter drain() when a tick callback spins wait_for_promise. + if (globalObject->m_isDrainingNextTickQueue) + return; + auto queue = globalObject->m_nextTickQueue.get(); + if (!queue || queue->isEmpty()) + return; + WTF::SetForScope drainingGuard(globalObject->m_isDrainingNextTickQueue, true); + queue->drain(vm, globalObject); } static void cleanupAsyncHooksData(JSC::VM& vm) { auto* globalObject = defaultGlobalObject(); globalObject->m_asyncContextData.get()->putInternalField(vm, 0, jsUndefined()); - globalObject->asyncHooksNeedsCleanup = false; - if (!globalObject->m_nextTickQueue) { - vm.setOnEachMicrotaskTick(&checkIfNextTickWasCalledDuringMicrotask); - checkIfNextTickWasCalledDuringMicrotask(vm); - } else { - vm.setOnEachMicrotaskTick(nullptr); - } + vm.setOnEachMicrotaskTick(&checkIfNextTickWasCalledDuringMicrotask); + checkIfNextTickWasCalledDuringMicrotask(vm); } GlobalObject* GlobalObject::create(JSC::VM& vm, JSC::Structure* structure) @@ -445,16 +445,7 @@ JSC::Structure* GlobalObject::createStructure(JSC::VM& vm) void Zig::GlobalObject::resetOnEachMicrotaskTick() { - auto& vm = this->vm(); - if (this->asyncHooksNeedsCleanup) { - vm.setOnEachMicrotaskTick(&cleanupAsyncHooksData); - } else { - if (this->m_nextTickQueue) { - vm.setOnEachMicrotaskTick(nullptr); - } else { - vm.setOnEachMicrotaskTick(&checkIfNextTickWasCalledDuringMicrotask); - } - } + this->vm().setOnEachMicrotaskTick(&cleanupAsyncHooksData); } extern "C" size_t Bun__reported_memory_size; @@ -555,15 +546,7 @@ extern "C" JSC::JSGlobalObject* Zig__GlobalObject__create(void* console_client, vm.setOnComputeErrorInfo(computeErrorInfoWrapperToString); vm.setOnComputeErrorInfoJSValue(computeErrorInfoWrapperToJSValue); vm.setComputeLineColumnWithSourcemap(computeLineColumnWithSourcemap); - vm.setOnEachMicrotaskTick([](JSC::VM& vm) -> void { - // if you process.nextTick on a microtask we need this - auto* globalObject = defaultGlobalObject(); - if (auto queue = globalObject->m_nextTickQueue.get()) { - globalObject->resetOnEachMicrotaskTick(); - queue->drain(vm, globalObject); - return; - } - }); + vm.setOnEachMicrotaskTick(&checkIfNextTickWasCalledDuringMicrotask); if (executionContextId > -1) { const auto initializeWorker = [&](WebCore::Worker& worker) -> void { @@ -3252,7 +3235,7 @@ uint8_t GlobalObject::drainMicrotasks() } scope.assertNoExceptionExceptTermination(); - if (auto nextTickQueue = this->m_nextTickQueue.get()) { + if (auto nextTickQueue = this->m_nextTickQueue.get(); nextTickQueue && !nextTickQueue->isEmpty()) { nextTickQueue->drain(vm, this); if (auto* exception = scope.exception()) { if (vm.isTerminationException(exception)) { diff --git a/src/jsc/bindings/ZigGlobalObject.h b/src/jsc/bindings/ZigGlobalObject.h index f58ed618a745..805cf2eebe87 100644 --- a/src/jsc/bindings/ZigGlobalObject.h +++ b/src/jsc/bindings/ZigGlobalObject.h @@ -433,7 +433,7 @@ class GlobalObject : public Bun::GlobalScope { return func; } - bool asyncHooksNeedsCleanup = false; + bool m_isDrainingNextTickQueue = false; double INSPECT_MAX_BYTES = 50; bool isInsideErrorPrepareStackTraceCallback = false; diff --git a/test/regression/issue/34115.test.ts b/test/regression/issue/34115.test.ts new file mode 100644 index 000000000000..a7f346b8ebef --- /dev/null +++ b/test/regression/issue/34115.test.ts @@ -0,0 +1,162 @@ +// https://github.com/oven-sh/bun/issues/34115 +// A preload script that touches process.nextTick (directly, or via a module that +// does) must not change the relative order of process.nextTick vs microtasks +// scheduled at the top level of the entry module. +// +// node:worker_threads workers hit the same path: since #31216 every such worker +// implicitly preloads node:worker_threads, which pulls in node:stream and +// touches process.nextTick before the worker's own entry runs. +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +const orderFixture = ` +process.nextTick(() => console.log("nextTick")); +queueMicrotask(() => console.log("microtask")); +Promise.resolve().then(() => console.log("promise")); +`; + +describe("process.nextTick ordering is preserved with --preload", () => { + test.concurrent.each([ + ["that reads process.nextTick", `process.nextTick;`], + ["that calls process.nextTick", `process.nextTick(() => console.log("preload-tick"));`, "preload-tick\n"], + ["that requires node:stream", `require("node:stream");`], + ["that requires node:zlib", `require("node:zlib");`], + ])("preload %s", async (_name, preloadBody, preloadOutput = "") => { + using dir = tempDir("issue-34115-order", { + "preload.js": preloadBody, + "order.js": orderFixture, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "--preload", "./preload.js", "order.js"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe(preloadOutput + "nextTick\nmicrotask\npromise\n"); + expect(exitCode).toBe(0); + }); + + test.concurrent("preserved with two preload scripts", async () => { + using dir = tempDir("issue-34115-two-preloads", { + "preload-a.js": `process.nextTick(() => console.log("a"));`, + "preload-b.js": `process.nextTick(() => console.log("b"));`, + "order.js": orderFixture, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "--preload", "./preload-a.js", "--preload", "./preload-b.js", "order.js"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("a\nb\nnextTick\nmicrotask\npromise\n"); + expect(exitCode).toBe(0); + }); +}); + +describe("process.nextTick ordering at the top level of a worker_threads CJS entry", () => { + const workerOrderBody = ` +const seq = ["sync"]; +Promise.resolve().then(() => seq.push("pt")); +queueMicrotask(() => seq.push("qm")); +process.nextTick(() => seq.push("nt")); +setImmediate(() => require("node:worker_threads").parentPort.postMessage(seq.join(","))); +`; + + test.concurrent.each([ + ["file entry", `"./worker.cjs"`], + ["eval: true", `${JSON.stringify(workerOrderBody)}, { eval: true }`], + ])("matches the main thread (%s)", async (_name, workerArgs) => { + using dir = tempDir("issue-34115-worker", { + "worker.cjs": workerOrderBody, + "main.mjs": ` +import { Worker } from "node:worker_threads"; +const w = new Worker(${workerArgs}); +w.on("message", seq => { console.log(seq); w.terminate(); }); +w.on("error", e => { console.error(String(e)); process.exitCode = 1; }); +`, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.mjs"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("sync,nt,pt,qm\n"); + expect(exitCode).toBe(0); + }); +}); + +test.concurrent("Writable.toWeb() close rejects with ABORT_ERR when preload requires node:stream", async () => { + using dir = tempDir("issue-34115-writable", { + "preload.js": `require("node:stream");`, + "repro.js": ` + const { Writable } = require("stream"); + const w = new Writable({ write(c, e, cb) { cb(); } }); + const ws = Writable.toWeb(w); + ws.close().then( + () => console.log("RESOLVED"), + e => console.log("rejected:", e.code), + ); + w.end(); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "--preload", "./preload.js", "repro.js"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("rejected: ABORT_ERR\n"); + expect(exitCode).toBe(0); +}); + +// A nextTick callback that spins wait_for_promise re-enters GlobalObject::drainMicrotasks +// under the hook's m_isDrainingNextTickQueue guard; JSNextTickQueue::drain must not clear +// asyncContextData[0] on that nested path. +test.concurrent("AsyncLocalStorage frame survives a nextTick callback that spins wait_for_promise", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` +const { AsyncLocalStorage } = require("node:async_hooks"); +const als = new AsyncLocalStorage(); +als.run("outer-frame", () => { + process.nextTick(() => { + const before = als.getStore(); + new HTMLRewriter() + .on("*", { + async element() { + await 0; + process.nextTick(() => {}); + await 0; + }, + }) + .transform(new Response("
")) + .text(); + console.log(before, als.getStore()); + }); +}); +`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("outer-frame outer-frame\n"); + expect(exitCode).toBe(0); +});