[robobun]

Related to #44116

Problem

• After a ShadowRealm loads Bun.SQL, a Postgres query of the first realm resolves to undefined. The process reports TypeError: undefined is not an object (evaluating 'is_last') from onResolvePostgresQuery. A MySQL query never settles.
• init of each adapter stores its callbacks in per-VM Strongs (RuntimeState.sql_rare). The last realm that calls init wins.
• Under bun test --isolate queries settle, and the only effect is one finished file's global kept alive. No user hit this: Bun.SQL: a ShadowRealm that reads Bun.SQL takes over the query callbacks of every realm #44116 was closed as not planned.

Fix

• init stores the callbacks in five GC slots of its realm's global (m_sqlCallbacks). The readers use the global of their connection or query.
• Delete RuntimeState.sql_rare, its hook and both context files.
• Verified: 5 new cases in isolation.test.ts, sql-handles.test.ts and postgres-bind-wire.test.ts. All fail on bun test --isolate: give each test file its own Bun.s3 client #47493.
• Self-reviewed with bun test --isolate: give each test file its own Bun.s3 client #47493: 25 concerns raised, 23 addressed (Notes).

Background

• A realm is a global object with its own modules: one per --isolate test file, one per ShadowRealm.
• REVIEW.md says no new fields on ZigGlobalObject. sql: consolidate the open Bun.sql, bun:sqlite and node:sqlite pull requests #45304 put the per-realm SQL error structures there.
• Smaller fix: clear the five Strongs at the file swap. It leaves the ShadowRealm cases.
• Considered the internal module registry as the home: no new field, but a property lookup for each result.

Downsides

• Every realm grows by 40 bytes and 5 GC-visited slots, with or without bun:sql: sizeof(Zig::GlobalObject) 10352 to 10392.
• Still per VM: require.extensions handlers (bun test --isolate: reset Module._extensions per file and ask it before the source cache answers a require() #47477), node:quic callbacks, node:cluster child state.

Notes

Stacked on #47493. The base branch is the branch of #47493.

Repro (local Postgres)

const sql = new Bun.SQL("postgres://postgres@127.0.0.1:5432/bun_sql_test", { max: 1 });
await sql`select 1 as x`; // [{ x: 1 }]
new ShadowRealm().evaluate("void Bun.SQL");
await sql`select 2 as x`; // main: undefined and the TypeError above. This PR: [{ x: 2 }]

The other direction fails too on main: a ShadowRealm that made its client first loses its results when the host realm loads bun:sql. An array parameter of the first realm goes out with the wrong text: a Date in a timestamptz[] is sent as {"\"2000-01-01T00:00:01.000Z\""}, because value instanceof Date runs in the other realm.

Measurements (linux-x64, #47493 against this PR)

What	Before	After	How
sizeof(Zig::GlobalObject)	10352 B	10392 B	lldb, debug builds
RuntimeState		40 B smaller for each VM	five pointers
GC member rows of the global	140	141 (+5 slots)	rows of FOR_EACH_GLOBALOBJECT_GC_MEMBER
Full collection, Bun.gc(true)	776,397 instructions	776,156 instructions	instructions:u over 2000 collections, one marker, JIT off, 3 interleaved rounds. The spread between rounds is 0.5%. The 5 slots are below it.
Callback lookup in PostgresSQLQuery::on_result	16 instructions, 1 call, 3 loads	4 instructions, no call, 1 load	objdump, release. SQL__realmCallback has no symbol: LTO inlined it.
Protected roots after void Bun.sql	Function 5	none	heapStats().protectedObjectTypeCounts
Live globals in the third isolated file, after file 1 read Bun.sql	3	2, as with no read	heapStats() after two Bun.gc(true), --isolate and --parallel=2, release
.text	108,797,673 B	108,797,161 B (-512 B)	size -A. .rodata does not change. No new host function.

The collection and lookup rows were measured on main. Its SQL code is the same as #47493's.

perf, valgrind, strace and bloaty are not installed in the container. The instruction counts come from perf_event_open.

Tests

• isolation.test.ts: after a file that read Bun.sql, or every property of Bun, the next file takes a heap snapshot and finds no root that reaches the global of the finished file. On bun test --isolate: give each test file its own Bun.s3 client #47493 it prints StrongRootBlock {root: StrongHandles} -> Function "onResolvePostgresQuery" -> Structure "Function" -> GlobalObject. The second row is there to catch the next root of this kind.
• The root walk moved from module-graph-isolation.test.ts to test/js/bun/gc/realms-a-root-reaches.fixture.js. Both tests use it.
• sql-handles.test.ts, for Postgres and MySQL: the queries of the host realm and of a ShadowRealm settle in their own realm, whichever loaded bun:sql last.
• postgres-bind-wire.test.ts: an array of Dates has the same Bind text after a ShadowRealm loads bun:sql.
• All 89 files of test/js/sql on local Postgres 17 and MariaDB, debug builds: main 4403 pass and 61 fail, this PR 4406 pass and 58 fail. The difference is the 3 new cases. The 58 need TLS or auth containers that this machine does not have, and fail the same way on main.
• Also run: all of isolation.test.ts (53 pass), sql-handles.test.ts (260 pass), postgres-bind-wire.test.ts (62 pass), module-graph-isolation.test.ts (413 pass).

The smaller fix, in detail. swap_global_for_test_isolation already clears node_quic_callbacks and the macros. The five SQL Strongs live in RuntimeState (bun_runtime), which bun_jsc cannot name, so the clear would go through a runtime hook. It must run after the last socket sweep: the Postgres readers .unwrap() the slots. Nobody built it.

Not changed

Other values of one realm that the VM or the process still keeps:

• require.extensions handlers (VirtualMachine.commonjs_custom_extensions): bun test --isolate: reset Module._extensions per file and ask it before the source cache answers a require() #47477.
• The node:quic callback holder (RareData.node_quic_callbacks). The file swap clears it. setCallbacks takes only the first call of a VM, so a second realm does not get its own.
• The node:cluster child state (CHILD_SINGLETON, src/runtime/node/node_cluster_binding.rs:28).

Self-review

The review covered #47493 and this PR as one diff, and asked for the split. From it: the test where the ShadowRealm runs a query, the array test, the root walk in place of a count of live objects, and this body. Not done: a fix for the JIT plan that can root a finished file's global for a short time (the retention test walks again when it sees one), and a move of the retention tests into the test table of another open PR.

[robobun]

^{Updated 2:06 PM PT - Oct 30th, 2026}

❌ @robobun, your commit 55be0ba has 4 failures in Build #135885 (All Failures):

• test/bundler/compile-aot-crash.test.ts - code 1 on 🍎 any x64
• step failed outside runner - exit 1 on 🍎 any x64 - test-bun
• test/js/bun/test/parallel/test-integration-rspack.ts - code 1 on 🪟 2019 x64
• test/js/bun/test/parallel/test-integration-rspack.ts - code 1 on 🪟 11 aarch64
• test/aot/engine/ir-6.test.ts - code 1 on 🐧 3.23 aarch64
• test/aot/engine/ir-6.test.ts - code 1 on 🐧 13 x64-asan
• test/aot/engine/ir-6.test.ts - code 1 on 🐧 3.23 x64
• test/aot/engine/ir-6.test.ts - code 1 on 🪟 2019 x64
• test/aot/engine/ir-6.test.ts - code 1 on 🪟 11 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 47494

That installs a local version of the PR into your bun-47494 executable, so you can run:

bun-47494 --bun

[robobun]

Status

• Reproduced on main (f7607a3d35) with a local Postgres: after new ShadowRealm().evaluate("void Bun.SQL"), the next query of the first realm resolves to undefined and the process reports TypeError: undefined is not an object (evaluating 'is_last'). With a mock MySQL server the query never settles.

• Reproduced the retention with bun test --isolate: after a file that read Bun.sql, a heap snapshot in the next file shows StrongRootBlock -> Function "onResolvePostgresQuery" -> GlobalObject of the finished file.

• Pull request: bun:sql: keep the query callbacks on the realm that registers them #47494. It is on top of bun test --isolate: give each test file its own Bun.s3 client #47493.

• CI: build 135885 finished with 179 of 188 jobs green. The red jobs fail on three tests that this change does not touch:

  • test/aot/engine/ir-6.test.ts fails the same way on the head commit of main (build 135385).
  • test/js/bun/test/parallel/test-integration-rspack.ts on Windows: the npm registry answered 404 for tslib-2.9.1.tgz.
  • test/bundler/compile-aot-crash.test.ts on macOS x64: the test looks for the word sum in the crash report URL, and the encoded addresses of this build contain those letters by chance.

  One macOS x64 step also failed outside the test runner. The other failures passed on a retry.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked the remaining .unwrap() reads of the postgres slots (PostgresSQLQuery.rs resolve/reject, PostgresRequest.rs array_literal): a query or connection can only be created through the adapter module whose init already filled that realm's slots, so an empty slot on the query's own global is not reachable from user code. The uncheckedDowncast<Zig::GlobalObject> in SQLClient.cpp matches the sibling SQL error-structure accessors on the same global, and the new std::array member is walked by the existing GC member V-list.

Extended reasoning...

The change moves the five bun:sql adapter callbacks from per-VM Strong handles in RuntimeState into a per-realm std::array of WriteBarriers on Zig::GlobalObject, with two new extern "C" entry points and a repr(u8) enum on the Rust side, deleting the two context files and the sql_rare hook. It touches GC rooting and a global-object layout but no auth, injection, or data-exposure surface. The two inline findings are minor (an undrained stdout pipe in a spawn test and the new field on ZigGlobalObject that the repository rules discourage), and the behavior-preservation of the old unwrap/early-return paths was traced through the adapter init ordering.

[claude]

🟡 nit (optional): every realm on the VM now carries 5 SQL-specific GC slots on Zig::GlobalObject, which the repository review rules say should not gain new fields. The V-list entry at src/jsc/bindings/ZigGlobalObject.h:470 adds m_sqlCallbacks for all globals whether or not bun:sql ever loads there. Fix: keep the per-realm callback storage with the feature, for example a single lazily created per-realm SQL cell (or the realm's own bun:sql module object) that SQL__setRealmCallback and SQL__realmCallback reach without adding adapter-specific members to the global. The PR notes the 40-byte, 5-slot growth as accepted and cites m_mySQLErrorStructure as precedent; that precedent is one slot per adapter, this adds five.

Why this was flagged

The diff adds V(public, SQLCallbacksArray, m_sqlCallbacks) to FOR_EACH_GLOBALOBJECT_GC_MEMBER at src/jsc/bindings/ZigGlobalObject.h:470 with SQLCallbacksArray = std::array<WriteBarrier, 5> at line 367. Every Zig::GlobalObject (main realm, each --isolate file realm, each ShadowRealm) grows by 40 bytes and five slots that visitChildrenImpl walks via visitGlobalObjectMember at src/jsc/bindings/ZigGlobalObject.cpp:3677, even when bun:sql is never imported in that realm. The base branch stores these callbacks outside the global (RuntimeState.sql_rare). The repository review guidance lists 'no new fields on ZigGlobalObject' under Architecture & layering as an explicit check; the PR description acknowledges the rule and the size cost and chose to proceed, so this is a design nit for the maintainer rather than a runtime defect.

Verification: Every Zig::GlobalObject constructed carries the slots whether or not bun:sql loads there. src/jsc/bindings/ZigGlobalObject.h:470 adds V(public, SQLCallbacksArray, m_sqlCallbacks) to FOR_EACH_GLOBALOBJECT_GC_MEMBER; src/jsc/bindings/ZigGlobalObject.cpp:3677 walks all five slots on every GC of every realm. The repository review instructions explicitly state "no new fields on ZigGlobalObject".

[robobun]

Not changed in this push. The description leaves this choice to a maintainer. The facts for it:

• Five slots: one load for each result. In PostgresSQLQuery::on_result the lookup is 16 instructions on main and 4 with this PR (objdump, linux-x64 release). The cost is 40 bytes and 5 visited slots in each realm.
• One holder cell, made when the first adapter loads: 8 bytes and 1 slot in each realm, and one more load for each result. It is still a new member. Not built.
• The module object of the realm, from the internal module registry: no member. Native code then finds each callback by property name for each result. Not built, so there is no count for it.
• sql: consolidate the open Bun.sql, bun:sqlite and node:sqlite pull requests #45304 added m_mySQLErrorStructure and m_postgresErrorStructure for the same reason: state of one realm that lived on the VM. This PR follows that with five slots where that one has two.

The holder variant changes only the two functions in SQLClient.cpp. The registry variant also removes init from the two .bind.ts files and exports the callbacks from the two JS modules.

[claude]

I re-reviewed the updated push and found no bugs; since this reshapes where bun:sql's native callbacks are rooted (a new GC-visited member on Zig::GlobalObject plus a new C ABI pair), a human look is still worthwhile.

What was reviewed:

• m_sqlCallbacks is reached by the GC member macro through the existing std::array<WriteBarrier<T>, n> overload in ZigGlobalObject.cpp, and WriteBarrier::set is called with the realm as owner.
• The Postgres .unwrap() sites: initPostgres runs at top level of src/js/internal/sql/postgres.ts, so a connection's realm has always registered its callbacks before on_result/reject/array_literal can run there; ShadowRealm globals are created via Zig::GlobalObject::create, so the unchecked downcast holds.
• No remaining references to sql_rare, sql_state, MySQLContext or PostgresSQLContext in src/; the RealmCallback enum is #[repr(u8)] with the five values the C++ array is sized for.
• The child-stdout drain in postgres-bind-wire.test.ts is now Promise.all over stdout, stderr and exit.

Extended reasoning...

The change moves the bun:sql resolve/reject/arrayLiteral callbacks from per-VM Strong handles in RuntimeState to five WriteBarrier slots on each Zig::GlobalObject, adds SQL__setRealmCallback/SQL__realmCallback in SQLClient.cpp and a RealmCallback enum in src/sql_jsc/jsc.rs, deletes MySQLContext.rs and PostgresSQLContext.rs, and rewires the three Postgres and two MySQL readers to look up by the query's own global. It touches no injection, auth or data-exposure surface; the sensitive part is GC rooting and FFI. The push since the prior review addressed the stdout-drain nit and trimmed comments; the design nit about adding a field to Zig::GlobalObject drew an author reply without a code change. The GC rooting and the unwrap preconditions check out, but the architectural choice and the native-code scope make this a change a maintainer should sign off rather than an automated approval.

Still open from earlier reviews (1):

• Unresolved: 1 minor or pre-existing.