Skip to content
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/js/builtins/ProcessObjectInternals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@

drainMicrotasks();
} while (!queue.isEmpty());
$putInternalField(nextTickQueue, 0, 0);

Check failure on line 390 in src/js/builtins/ProcessObjectInternals.ts

View check run for this annotation

Claude / Claude Code Review

23ee2728's 'mutually exclusive' revert justification is unsound: nested pTAR resets field(0) while outer guard is still up, so mustResetContext ALS-clear is reachable again

23ee2728 dropped a862f767's `!m_isDrainingNextTickQueue` guard on `JSNextTickQueue::drain`'s `mustResetContext` clear, arguing the flag and `internalField(0)==0` are mutually exclusive — but they aren't: a tick callback that spins `wait_for_promise` re-enters `GlobalObject::drainMicrotasks` → nested `processTicksAndRejections`, whose new line 390 resets field(0)=0 while the outer hook's `SetForScope` still holds the flag true. On the next spin iteration `drain()` enters with `isEmpty()` and flag
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
}

$putInternalField(nextTickQueue, 0, 0);
Expand Down
2 changes: 2 additions & 0 deletions src/jsc/bindings/BunProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
44 changes: 15 additions & 29 deletions src/jsc/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -389,23 +389,24 @@ 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.
Comment thread
robobun marked this conversation as resolved.
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)
Expand Down Expand Up @@ -445,16 +446,9 @@ 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);
}
}
// Sole caller (jsCleanupLater) sets asyncHooksNeedsCleanup = true immediately before.
ASSERT(this->asyncHooksNeedsCleanup);
this->vm().setOnEachMicrotaskTick(&cleanupAsyncHooksData);
}

extern "C" size_t Bun__reported_memory_size;
Expand Down Expand Up @@ -555,15 +549,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 {
Expand Down
1 change: 1 addition & 0 deletions src/jsc/bindings/ZigGlobalObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ class GlobalObject : public Bun::GlobalScope {
}

bool asyncHooksNeedsCleanup = false;
Comment thread
robobun marked this conversation as resolved.
Outdated
bool m_isDrainingNextTickQueue = false;
double INSPECT_MAX_BYTES = 50;
bool isInsideErrorPrepareStackTraceCallback = false;

Expand Down
123 changes: 123 additions & 0 deletions test/regression/issue/34115.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
// 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.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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);
});
Loading