Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
36 changes: 27 additions & 9 deletions src/sql_jsc/postgres/PostgresSQLQuery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,15 +479,6 @@ impl PostgresSQLQuery {
};
let connection: &PostgresSQLConnection = &connection;

// `KeepAlive::ref_` takes an `EventLoopCtx` (manual vtable in `bun_io`), not a
// `*mut VirtualMachine`. `global_object.bun_vm()` and `get_vm_ctx(.Js)` both
// resolve to the same singleton JS VM, so route through the global hook —
// identical to `PostgresSQLConnection::vm_ctx`.
connection.poll_ref.with_mut(|r| {
r.ref_(bun_io::posix_event_loop::get_vm_ctx(
bun_io::AllocatorType::Js,
))
});
let query = arguments[1];

if !query.is_object() {
Expand Down Expand Up @@ -561,6 +552,16 @@ impl PostgresSQLQuery {
return Err(global_object.throw_out_of_memory());
}

// Request is enqueued: keep the event loop alive until the server
// responds. KeepAlive is a flag (not a count), so taking this any
// earlier would leave it stuck Active on the synchronous-error
// returns above.
connection.poll_ref.with_mut(|r| {
r.ref_(bun_io::posix_event_loop::get_vm_ctx(
bun_io::AllocatorType::Js,
))
});

this.this_value.with_mut(|r| r.upgrade(global_object));
js::target_set_cached(this_value, global_object, query);
if this.status.get() == Status::Running {
Expand Down Expand Up @@ -630,6 +631,8 @@ impl PostgresSQLQuery {
Ok(v) => v,
Err(err) => {
drop(signature);
// SAFETY: undoes the speculative `this.ref_()` above; count was ≥2, never frees here.
unsafe { Self::deref(this_ptr) };
return Err(
global_object.throw_error(err.into(), "failed to allocate statement")
);
Expand Down Expand Up @@ -786,6 +789,8 @@ impl PostgresSQLQuery {
.with_mut(|m| m.remove(&signature_hash));
}
drop(signature);
// SAFETY: undoes the speculative `this.ref_()` above; count was ≥2, never frees here.
unsafe { Self::deref(this_ptr) };
if !global_object.has_exception() {
return Err(global_object.throw_value(postgres_error_to_js(
global_object,
Expand Down Expand Up @@ -854,8 +859,21 @@ impl PostgresSQLQuery {
.with_mut(|q| q.write_item(this_ptr))
.is_err()
{
this.release_statement();
// SAFETY: undoes the speculative `this.ref_()` above; count was ≥2, never frees here.
unsafe { Self::deref(this_ptr) };
return Err(global_object.throw_out_of_memory());
}
Comment thread
robobun marked this conversation as resolved.

// Request is enqueued: keep the event loop alive until the server
// responds. See the matching call in the simple-query branch above
// for why this must come after every fallible step.
connection.poll_ref.with_mut(|r| {
r.ref_(bun_io::posix_event_loop::get_vm_ctx(
bun_io::AllocatorType::Js,
))
});

this.this_value.with_mut(|r| r.upgrade(global_object));

js::target_set_cached(this_value, global_object, query);
Expand Down
58 changes: 58 additions & 0 deletions test/js/sql/sql-postgres-run-error-pollref-fixture.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 32 additions & 0 deletions test/js/sql/sql-postgres-run-error-pollref.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// PostgresSQLQuery.do_run refs the connection's poll_ref KeepAlive. KeepAlive
// is a two-state flag, not a counter, so when this query is the only in-flight
// work the call flips Inactive -> Active. When do_run then returns early with
// a synchronous error (bad binding, signature-generation failure, OOM during
// enqueue, ...) the poll_ref must not be left Active: nothing else on the
// connection will touch it until the next server message, so the event loop
// stays pinned and the process never exits.
//
// The fixture connects to a mock server, lets the connection go idle, then
// issues a query whose binding is rejected synchronously before anything is
// written. It must print the rejection and exit on its own.

import { expect, test } from "bun:test";
import { bunEnv, bunExe } from "harness";
import path from "node:path";

test("postgres: synchronous do_run failure does not pin the event loop", async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), path.join(import.meta.dir, "sql-postgres-run-error-pollref-fixture.ts")],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
void stderr;

expect(stdout).toBe("rejected:ERR_INVALID_ARG_TYPE\n");
// exited on its own, not killed by the runner's timeout
expect(proc.signalCode).toBeNull();
expect(exitCode).toBe(0);
});
Loading