sql(postgres): cap the prepared-statement cache and send Close on eviction - #33244
sql(postgres): cap the prepared-statement cache and send Close on eviction#33244robobun wants to merge 6 commits into
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Updated 2:39 AM PT - Jul 12th, 2026
❌ @robobun, your commit 898ae1a has 4 failures in
🧪 To try this PR locally: bunx bun-pr 33244That installs a local version of the PR into your bun-33244 --bun |
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
|
Checked both of these against the diff before linking them, since neither is an exact match: #28980: the part of that issue that kills the database is exactly this bug. Named statements are keyed on the parameter null-pattern, so #30010: that report is about I added a "Related issues" section to the PR description instead of |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a per-connection LRU cap for named PostgreSQL prepared statements, updates query and protocol handling, and adds wire helpers, regression tests, integration tests, and documentation. ChangesPrepared statement LRU cache
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I didn't find any bugs, but this changes wire-protocol behavior and adds unsafe intrusive-refcount eviction in the Postgres client, so it warrants a human look at the memory-safety and protocol-timing reasoning.
Extended reasoning...
Overview
This PR caps the per-connection Postgres prepared-statement cache at 256 entries with LRU eviction, sending Close('S', name) for evicted statements and reworking the CloseComplete handler to be a no-op instead of resolving the current request. It touches PostgresSQLConnection.rs (new get_or_put_statement / evict_lru_statements, new statement_lru_clock field, changed CloseComplete handling), PostgresSQLQuery.rs (routes cache lookup through the new helper), PostgresSQLStatement.rs (new last_used field and has_one_ref accessor), plus docs, a new 350-line test file, and additions to the shared wire-frames.ts test helpers.
Security risks
None apparent. This is client-side resource bookkeeping for outbound database connections; no auth, crypto, permissions, or untrusted-input parsing is affected. The Close message is standard Postgres protocol and only sent to a server the client already established a session with.
Level of scrutiny
This deserves careful human review. The eviction path uses unsafe { PostgresSQLStatement::deref(ptr.as_ptr()) } gated on has_one_ref(), relying on the invariant that no ref can be taken between the check and the deref (single-JS-thread, and protocol::Close::write only buffers bytes without re-entering JS). It also relies on get_or_put returning a stable value-slot pointer that survives until the later unsafe { *entry_value = stmt } store in do_run — which holds because eviction happens before get_or_put, but this is exactly the class of raw-pointer-into-container invariant CLAUDE.md flags for scrutiny. The protocol-timing argument (Close lands on an extended-query boundary, never mid-Parse/Bind/Execute/Sync) and the changed CloseComplete semantics both look right and are well-tested against both a scripted and a real server, but they are behavior changes to a production database client.
Other factors
The 256 cap is a hardcoded design choice (matching the MySQL side from #33190) rather than a configurable option — reasonable, but a maintainer should confirm. Test coverage is thorough (wire-level counters, real-Postgres pg_prepared_statements, JSC Structure leak assertion, reuse regression), and the PR description demonstrates before/after behavior. No prior human reviews on this PR yet.
|
CI status for this PR. Across the three most recent builds of this branch, no
I pushed one empty |
There was a problem hiding this comment.
I didn't find any bugs, but this touches intrusive-refcount lifetime management and Postgres wire-protocol message ordering (Close interleaved with pipelined extended-query sequences, plus the CloseComplete handler rewrite), so it's worth a human look.
Extended reasoning...
Overview
This PR caps the per-connection Postgres prepared-statement cache at 256 entries and sends Close('S', name) on LRU eviction. It adds get_or_put_statement / evict_lru_statements on PostgresSQLConnection, an LRU clock field, a last_used stamp and has_one_ref() on PostgresSQLStatement, refactors do_run to route through the new helper, and rewrites the CloseComplete handler to consume the ack without touching the request queue. Ships with a scripted wire-protocol test server, a real-Postgres container test, wire-frame helpers, and docs.
Security risks
None identified. No auth, crypto, or permission surface is touched; the change is internal cache management on an already-authenticated connection. Server-provided data is not newly parsed.
Level of scrutiny
High. This is production-critical database driver code, and the change sits squarely in the categories CLAUDE.md flags as most-blocked:
- Memory safety:
evict_lru_statementsiterates a map of raw*mut PostgresSQLStatement, checkshas_one_ref(), removes the entry, thenunsafe { deref() }s the last ref. The reasoning (single JS thread, no reentry between check and deref, map owns exactly one ref) looks sound but this is exactly the intrusive-refcount pattern that needs a second pair of eyes. - Protocol correctness: The Close is written into the connection's write buffer at the start of
do_run, before the new query's Parse/Bind. The PR argues this always lands on an extended-query message boundary, and the container test proves a real backend accepts it. But the interaction with pipelining, the lack of an explicit Sync after Close, and what happens when the subsequent enqueue fails after Close bytes are already buffered are subtle enough to warrant human review. - Behavior change: The
CloseCompletehandler previously resolved the current in-flight query with aCLOSECOMPLETEtag; it now silently consumes. The PR says the old path was unreachable (nothing sent Close before), which is plausible, but it's still a protocol-handler rewrite.
Other factors
- Test coverage is thorough: wire-level Parse/Close counting, a mock that fails Bind-to-closed-name loudly (SQLSTATE 26000), a heapStats-based Structure-rooting regression test, a real-Postgres
pg_prepared_statementscheck, and a reuse-not-evicted test. The PR verified fail-on-system-Bun / pass-on-debug-build. - Design choices a human should weigh: the hardcoded 256 cap (matches the MySQL side but is not user-configurable), and the O(n) linear scan per insert past the cap (fine at n=256 but worth a nod).
- No prior human reviews on the PR; only bot activity in the timeline.
5a7ff54 to
f1811c2
Compare
|
Rebased onto I kept that shape and adapted this PR to it instead of reverting:
Re-verified on the rebase: |
There was a problem hiding this comment.
I didn't find any bugs, but this touches refcount-driven eviction and Postgres wire-protocol handling in native code, so it's worth a human review of the memory-safety and pipelining assumptions.
Extended reasoning...
Overview
This PR bounds the per-connection Postgres prepared-statement cache at 256 entries with LRU eviction. On eviction it writes a Close('S', name) to the server and releases the client-side PostgresSQLStatement (and its rooted row Structure). It adds lookup_statement/put_statement/evict_lru_statements on PostgresSQLConnection, an LRU stamp and has_one_ref() on PostgresSQLStatement, rewires PostgresSQLQuery::do_run through the new helpers, and changes the CloseComplete handler to consume the ack without touching the request queue. Tests use a scripted wire-protocol server plus a real-Postgres container check; docs are updated.
Security risks
None apparent. No new user-controlled input is parsed; the only new bytes on the wire are a Close message built from the client's own statement name. No auth, TLS, or permission surface is touched.
Level of scrutiny
High. This is native Rust in a database driver with:
- Intrusive-refcount reasoning: eviction gates on
has_one_ref()and then doesunsafe { PostgresSQLStatement::deref(ptr) }after removing the map entry. The invariant that the map is the sole remaining owner at that point is subtle and depends on every other ref-holder (query wrappers, error paths) being accounted for. - Raw-pointer iteration over the
StringHashMapwhile separately borrowing each value viaParentRef, followed by awith_mutremove — the safety comments look right but this is exactly the pattern the CLAUDE.md memory-safety section flags for careful review. - A protocol-semantics change:
CloseCompletepreviously (unreachably) resolved the current request; it now no-ops. The argument that the Close is always written on an extended-query message boundary (before the new query's Parse/Bind/Execute) and never interleaves with a pipelined sequence deserves a human sanity-check against the surroundingdo_run/advanceflow.
Other factors
Test coverage is thorough (mock server enforces 26000/42P05 so a mis-timed Close would fail loudly; heapStats check for the client-side leak; real-Postgres convergence; reuse-doesn't-close regression), the bug-hunting pass found nothing, and the design mirrors the already-landed MySQL cap (#33190). But the combination of unsafe deref, refcount-based idleness detection, and a change to how a backend message is attributed to the request queue is beyond what I'd auto-approve without a maintainer's eyes.
…ction
Bun's Postgres client cached one named prepared statement per distinct query
text per connection and never sent Close: the statements map on
PostgresSQLConnection was insert-only and the Close protocol writer had no
callers. Every distinct query text on a long-lived connection permanently
retained a named prepared statement in the server session plus the client-side
statement, which roots a JSC Structure through a Strong handle.
Cap the per-connection cache at 256. Inserting a new entry past the cap evicts
the least recently used statement whose only remaining owner is the cache,
removes it from the map, and writes Close('S', name) so the server deallocates
it. Statements still referenced by a query are never evicted. The CloseComplete
reply is consumed without touching the request queue; the previous handler
attributed it to the current query, which would have corrupted a pipelined
result (it was unreachable before because nothing ever sent Close).
…victed statements The real-server test asserted pg_prepared_statements <= 256 immediately after re-running 300 texts. The cap is not enforced against statements whose query wrappers have not been collected yet, so on a fast release build (no natural GC during that loop) the count legitimately exceeds 256 and the assertion failed. Converge with the same GC + insert loop as the first phase before asserting, which also proves re-prepared entries are evictable again.
main added its own pgParseComplete/pgBindComplete/pgParameterDescription and a two-arg pgReadFrontendMessages in #33962; drop the duplicates this PR had introduced (keeping pgCloseComplete and pgReadCString, which main does not have) and rewrite the mock server to handle StartupMessage inline like the other fault-injection fixtures now do. Switch the real-server test to describeWithContainer (harness), matching the shape #33986 moved sql-prepare-false to so the test skips correctly on agents where docker is unavailable.
f1811c2 to
4153088
Compare
|
Rebased onto The bigger work was in the auto-merged files:
Re-verified on the rebase: 4/4 pass on |
There was a problem hiding this comment.
I didn't find any bugs, but this touches native memory management (intrusive refcounts, an unsafe deref on eviction) and the Postgres wire protocol in a production-critical client, so it warrants a human sign-off on the invariants the description spells out.
Checked: has_one_ref() gating eviction matches the two ref sites (init_exact_refs(2) on create, ref_() on cache hit) so the map's ref is provably last; protocol::Close has no other writer, so the CloseComplete no-op is safe; eviction runs before get_or_put, so the returned value-slot pointer isn't invalidated by a later map mutation.
Extended reasoning...
Overview
This PR caps the per-connection Postgres prepared-statement cache at 256 entries with LRU eviction, sending Close('S', name) to the server for each evicted statement. It touches three native Rust files in src/sql_jsc/postgres/ (the connection's statement map, the query enqueue path, and the statement's refcount/LRU stamp), rewrites the CloseComplete protocol handler to a no-op, adds a ~350-line scripted-server test, two small wire-frame helpers, and a docs paragraph.
Security risks
None in the traditional sense — no user-controlled input reaches new parsing or allocation paths. The risk surface is memory safety and protocol correctness: an unsafe PostgresSQLStatement::deref frees the evicted statement based on a has_one_ref() check, and Close bytes are injected into the write buffer between extended-query sequences. A bug in either would be use-after-free or wire-protocol corruption on a live database connection, not an injection or auth issue.
Level of scrutiny
High. This is native code in a database client with hand-rolled intrusive refcounting, an unsafe block whose SAFETY comment depends on a non-local invariant (the map owns exactly one ref per entry, and nothing between the has_one_ref() check and the deref can take a new one), and a change to wire-protocol reply attribution (CloseComplete no longer resolves the current request). The PR description itself notes that two prior review passes asked for a human check of exactly these invariants. Per the approval guidelines, complex changes to critical code paths with memory-safety implications should not be bot-approved.
Other factors
The change is well-tested (scripted server counting Parse/Close, a real-Postgres container test on pg_prepared_statements, a heap-stats leak test, and a reuse regression test), the invariants are carefully documented, and I verified the supporting claims: protocol::Close has exactly one writer (the new eviction path), so the CloseComplete no-op cannot orphan any other flow; the value-slot pointer returned by put_statement is captured after eviction completes, so the map is not mutated between capture and store; and the refcount accounting (2 refs on create, +1 on cache hit, map owns 1) makes has_one_ref() a sound idleness check. I found nothing wrong — but the combination of unsafe deref-on-eviction, wire-protocol reply handling, and the O(n) scan-per-insert design choice (acceptable at n≤256, but a design decision) is squarely in "a human should confirm" territory.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/runtime/sql.mdx`:
- Around line 988-989: Update the prepared-statement cache documentation to say
statements referenced by query objects are not evicted, replacing the narrower
“in-flight queries” wording. Ensure the temporary cache-limit exceedance
description matches the implementation’s handling of retained completed
PostgresSQLQuery objects.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6b81d16e-abf5-4058-b4ff-122412b5942b
📒 Files selected for processing (1)
docs/runtime/sql.mdx
…t land inside a re-entered Bind A bound value's toJSON/valueOf/toString can synchronously re-enter do_run on the same connection while an outer write_bind() is between writing 'B' and patching the length. Writing Close to write_buffer there would be folded into that Bind's Int32 length and rejected with 08P01. Gate evict_lru_statements on !has_query_running() so it only writes at the same point the new query itself would; eviction defers to the next insert, which is already the behavior when no idle victim exists.
There was a problem hiding this comment.
The has_query_running() gate in 898ae1a addresses the re-entrancy concern I raised — Close now defers when a write is mid-frame. No new findings this pass, but this touches native refcount/unsafe-deref invariants and wire-protocol framing in the Postgres client, so it warrants a human reviewer.
Checked: the eviction loop's has_one_ref() → remove → unsafe deref sequence has no JS re-entry between the check and the free (protocol::Close::write only appends bytes); the null-slot panic under re-entrant put_statement was examined and ruled out — on the miss path, nothing between get_or_put and the slot store runs user JS (!has_params ⇒ no coercion in prepare_and_query_with_signature; the params path only writes Parse/Describe/Sync). CloseComplete is now correctly decoupled from the request queue.
Extended reasoning...
Overview
This PR caps the per-connection Postgres prepared-statement cache at 256 entries (matching the MySQL side from #33190) and sends Close('S', name) on LRU eviction. Changes: ~130 lines of native Rust in PostgresSQLConnection.rs (new lookup_statement/put_statement/evict_lru_statements, rewritten CloseComplete handler), small additions to PostgresSQLStatement.rs (last_used LRU stamp, has_one_ref), a refactor of the cache-miss path in PostgresSQLQuery.rs to route through the new helpers, a 351-line scripted-server + real-Postgres test, two wire-frame helpers, and a docs paragraph.
Prior review
My previous run flagged that evict_lru_statements() wrote Close to write_buffer unconditionally, so a re-entrant do_run (bound value's toString/valueOf calling .execute() mid-write_bind) could interleave Close bytes into an open Bind frame. Commit 898ae1a added an early-return on has_query_running(), which is the same gate the new query's own write is behind. I re-traced the scenario: when the outer writer is advance(), the peeked query is in the FIFO so the gate holds; when the outer is a cache-hit do_run calling bind_and_execute, the inner takes the hit path (no put_statement), so no eviction write. The remaining sibling hazard on that path is pre-existing and out of scope, as noted in the thread.
The bug hunting system also raised and refuted a null-map-slot panic in evict_lru_statements under re-entrancy: on the cache-miss path, between get_or_put returning the slot pointer and the store into it, the only calls that run are prepare_and_query_with_signature (only when !has_params, so no bound-value coercion) or write_query (Parse/Describe/Sync only). Neither reaches user JS, so no re-entrant iteration sees the null slot.
Security risks
None identified. This is client-side cache management and outbound protocol framing; no auth, crypto, or untrusted-input parsing changes. The Close message carries only a statement name the client itself minted.
Level of scrutiny
High. This is production native code in a database client with an unsafe deref whose soundness rests on an intrusive-refcount invariant (has_one_ref() ⇒ map owns the last ref), plus wire-protocol placement constraints. The PR description spells out three invariants and the tests exercise them (scripted server rejects Bind-to-closed-name with 26000, so a premature Close fails loudly), but per repo guidance, refcount balancing and unsafe-deref soundness in this codebase are exactly the categories that most often block merges and should get maintainer eyes (cirospaciari was suggested).
Other factors
Test coverage is thorough: scripted server counting Parse/Close, a heapStats-based Structure-count regression, a real-Postgres pg_prepared_statements convergence check via describeWithContainer, and a reuse-never-closes sanity test. All four fail on released Bun and pass on the debug build per the PR evidence. CI on 898ae1a is building; prior builds passed the SQL suites. The one CodeRabbit docs nit is resolved. No outstanding reviewer comments.
1 participant
Problem
The Postgres client caches one named prepared statement per distinct query text per connection and never deallocates any of them: the
statementsmap onPostgresSQLConnectionis insert-only, and theCloseprotocol writer (src/sql/postgres/protocol/Close.rs) had no callers. This is the Postgres side of #33190 (MySQL), which left Postgres alone.Named prepared statements live in the server session until they are closed or the connection ends, so one long-lived pooled connection running many distinct query texts (ORMs interpolate identifiers and column lists into SQL constantly) grows the backend's prepared-statement catalog monotonically for the life of the connection. The same defect leaks client memory: each cached
PostgresSQLStatementkeeps its metadata and roots one JSCStructure(its row shape) through aStronghandle until the connection closes.Measured against a scripted Postgres server with Bun 1.4.0 (one connection, distinct query texts,
Bun.gc(true)between samples):bun:jscheapStats().protectedObjectTypeCounts.Structuregrows by exactly one per distinct text (+300 after 300 texts) and nothing is ever released; the wire capture shows 301Parse, 0Close. Against a real PostgreSQL (same probe, one connection,pg_prepared_statementsqueried over the simple protocol so the probe does not perturb itself):prepare: falseavoids it (unnamed statements), but that gives up pipelining and per-statement planning, and nothing bounds the default path.Fix
src/sql_jsc/postgres/PostgresSQLConnection.rs: cap the per-connection cache atMAX_CACHED_PREPARED_STATEMENTS(256, same constant as the MySQL side in sql(mysql): cap the prepared-statement cache and send COM_STMT_CLOSE on eviction #33190). Inserting a new entry past the cap evicts the least recently used statement whose only remaining owner is the cache, removes it from the map, and writesClose('S', name)for it. A statement still referenced by a query object (pending, running, or not yet collected) is never evicted, so a name can never be closed out from under a query that will still bind to it; the cache defers past the cap instead. Only statements the server acknowledged (statusPrepared) get aClose. The eviction is written at the start of the new query's enqueue, so it always lands on an extended-query message boundary, never inside another statement's Parse/Bind/Execute/Sync sequence.MessageType::CloseComplete: consume the acknowledgement without touching the request queue. The previous handler attributed CloseComplete to the current in-flight query and resolved it with a bogusCLOSECOMPLETEcommand tag; it was unreachable before (nothing ever sent Close) but would have corrupted a pipelined result once eviction exists.postgres.jsalso treats CloseComplete as a no-op.src/sql_jsc/postgres/PostgresSQLStatement.rs: add thelast_usedLRU stamp, bumped when a later query reuses a cached statement, and thehas_one_refidleness check.docs/runtime/sql.mdx: document the cap in the prepared statements section.Evicting releases the client side as well: the map drops its (sole) ref, freeing the
PostgresSQLStatementand theStrongrooting its cached rowStructure, so client memory is bounded by the cap instead of growing with every distinct query text.Verification
test/js/sql/sql-postgres-statement-cache.test.tsdrives a scripted Postgres server (frame builders added totest/js/sql/wire-frames.ts) that countsParse/Close('S')per statement name and, like a real server, answers aBindto a closed or unknown name with SQLSTATE 26000 and aParsethat redefines a live name with 42P05, so closing a statement another query still needed fails that query loudly.Closeis sent and live server statements (parses minus closes) converge back under the cap. Unfixed, 0 closes are ever sent.Structurecount (heapStats().protectedObjectTypeCounts.Structure) stays within the cap after exceeding it. Unfixed, it grows by one per distinct text.postgres_plaincontainer, skipped where Docker is unavailable):select count(*) from pg_prepared_statementson the session converges to <= 256 after 300 distinct texts, and all 300 texts still run correctly afterwards (evicted ones re-prepare). This also proves a real backend accepts the Close where the client writes it.Parse, 0Close(the cache still reuses).The other Postgres suites (
sql.test.tsnon-container tests,postgres-invalid-message-length,postgres-multi-statement-fields,postgres-simple-query-pipeline,postgres-binary-*,postgres-failed-connection-resurrection,postgres-tls-ctx-leak,sql-prepare-false,sql-connect-error-reporting,sql-close-pending-connection,wire-frames) still pass with the debug build.Related issues (not closed by this PR)
Bun.SQLbulk-insert rows with nullable columns causes database to crash #28980: the database-side OOM there is this unbounded accumulation (named statements are keyed on the parameter null-pattern, sosql(rows)batches with nullable columns mint a new statement on almost every batch). This PR bounds the damage: the session never holds more than 256 named statements, so the server no longer grows until it dies. It does not change the cache key, so the same SQL text with a different null-pattern still re-prepares (verified: 2 statements for one INSERT text with a random null after this PR). Making those hit one statement is a separate change toSignature::generate.PostgresSQLConnectionwrappers reported there retain their whole statement cache, so this cap shrinks what each leaked connection holds, but it does not fix the wrapper retention itself. The cached rowStructurecannot be what roots the wrapper (JSC__createStructureonly uses the owner for a write barrier), so that leak needs its own investigation.Rebase onto #33072 ("Hardening round 11")
That PR rekeyed the Postgres statement cache from
HashMap<u64 /* wyhash */, …>toStringHashMap<…>(keyed on the signature bytes, to stop hash collisions aliasing statements) and split the lookup into a zero-allocation hit probe followed byget_or_putonly on a miss. This PR is rebased onto that shape rather than reverting it:get_or_put_statement(u64)is gone. It is nowlookup_statement(&[u8])(the hit probe, which also stamps the LRU clock) andput_statement(&[u8])(the miss path, which evicts under the cap and returns the map's value-slot pointer).evict_lru_statementsremoves the victim bystmt.signature.name, the same key the statement owns a copy of and that theErrorResponsearm already removes by.Close('S', name)for those the server acknowledged.Re-verified after the rebase: 4/4 pass on the debug build (3 of them fail on released Bun), the real-Postgres probe still converges to 256 named statements after 300 distinct query texts, and the neighboring Postgres suites (86 tests across 9 files) are green.
Invariants the eviction relies on
Spelled out since this is native code in a database client, and two reviews have asked for a human check of exactly these:
PostgresSQLStatementgets an intrusive ref in exactly two places:stmt.ref_()when a query hits it in the cache, andinit_exact_refs(2)when a query creates it (one ref for the query, one for the map). The cache itself owns exactly one ref per entry, sohas_one_ref()(refcount == 1) holds iff no query object references the statement, i.e. nobody can still send aBindnaming it. Eviction skips everything else, and defers past the cap rather than closing a live name.unsafe derefon eviction is balanced and cannot race a new ref. The only code that takes a statement ref is the cache probe indo_run, and eviction runs insidedo_runafter that probe already missed for the new key. The only call between thehas_one_ref()check and thederefisprotocol::Close::write, which appends bytes towrite_bufferand cannot re-enter JS. So the ref the map owned is the last one, and removing the entry transfers it to thederefthat frees the statement.Closealways lands on a frontend-message boundary. Eviction only writes when!has_query_running(), the same gate the new query's own Parse/Bind is behind. That also rules out the one re-entrancy path that would otherwise violate this: a bound value'svalueOf/toJSON/toStringre-enteringdo_runon the same connection while an outerwrite_bindis between writing'B'and patching its Int32 length. With the gate, the Close is written before the new query'sParseon an otherwise-quiescent write buffer, so it is never interleaved into another statement's Parse/Bind/Execute/Sync sequence (and it cannot target a statement in such a sequence, by invariant 1). The backend'sCloseCompleteis flushed together with that query'sFlush/Syncand consumed without touching the request queue. If the enqueue then fails after theClosebytes are buffered, the state stays consistent: the server frees the statement, the cache entry is already gone, the client statement is already freed, and theCloseCompleteis still consumed as a no-op.[review] gate passed · iteration 8 · 6 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 3 passed · 0 rejected · iteration 8
evidence per changed file