Skip to content
Open
Show file tree
Hide file tree
Changes from 10 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 @@ export function initializeNextTickQueue(

drainMicrotasks();
} while (!queue.isEmpty());
$putInternalField(nextTickQueue, 0, 0);
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
4 changes: 1 addition & 3 deletions src/jsc/bindings/NodeAsyncHooks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Zig::GlobalObject>(globalObject);
global->asyncHooksNeedsCleanup = true;
global->resetOnEachMicrotaskTick();
uncheckedDowncast<Zig::GlobalObject>(globalObject)->resetOnEachMicrotaskTick();
return JSC::JSValue::encode(JSC::jsUndefined());
}

Expand Down
45 changes: 14 additions & 31 deletions src/jsc/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -389,23 +389,23 @@
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 +445,7 @@

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;
Expand Down Expand Up @@ -555,15 +546,7 @@
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 Expand Up @@ -3252,7 +3235,7 @@
}
scope.assertNoExceptionExceptTermination();

if (auto nextTickQueue = this->m_nextTickQueue.get()) {
if (auto nextTickQueue = this->m_nextTickQueue.get(); nextTickQueue && !nextTickQueue->isEmpty()) {

Check warning on line 3238 in src/jsc/bindings/ZigGlobalObject.cpp

View check run for this annotation

Claude / Claude Code Review

mustResetContext branch in JSNextTickQueue::drain is now dead after ee301a4

Nit: with both hot-path callers now gated on `!isEmpty()`, `mustResetContext` and the `m_asyncContextData[0]` clear at JSNextTickQueue.cpp:90-93 are provably unreachable — the only remaining ungated caller (`Process__dispatchOnBeforeExit`) enters with the persistent hook armed, so any nextTick queued during line 83's `vm.drainMicrotasks()` is drained (and field(0) reset to 0) by a nested pTAR before line 88 is checked. The earlier review this addresses suggested "drop the clear entirely, since w
Comment thread
robobun marked this conversation as resolved.
nextTickQueue->drain(vm, this);
if (auto* exception = scope.exception()) {
if (vm.isTerminationException(exception)) {
Expand Down
2 changes: 1 addition & 1 deletion src/jsc/bindings/ZigGlobalObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
162 changes: 162 additions & 0 deletions test/regression/issue/34115.test.ts
Original file line number Diff line number Diff line change
@@ -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.
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);
});

// A nextTick callback that spins wait_for_promise re-enters GlobalObject::drainMicrotasks
// while the hook's re-entrancy guard is held; gating that call on !isEmpty() keeps the
// nested drain from clearing asyncContextData[0] via the mustResetContext branch.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment thread
robobun marked this conversation as resolved.
Outdated
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("<div></div><div></div>"))
.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);
});
Loading