From 258a2a2e3adfe466f1e9c52266977a36781ecd8a Mon Sep 17 00:00:00 2001
From: Ciro Spaciari <ciro.spaciari@gmail.com>
Date: Mon, 4 Aug 2025 19:41:20 -0700
Subject: [PATCH] fix(postgres) memory fix when connection fails sync (#21616)

### What does this PR do?
We should not call .deinit() after .toJS otherwise hasPendingActivity
will access invalid memory

### How did you verify your code works?
Test run it with debug build on macos or asan on and will catch it

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
---
 src/sql/postgres/PostgresSQLConnection.zig | 21 ++++++++++-----------
 test/js/sql/socket.fail.fixture.ts         | 19 +++++++++++++++++++
 test/js/sql/sql.test.ts                    | 15 ++++++++++++++-
 3 files changed, 43 insertions(+), 12 deletions(-)
 create mode 100644 test/js/sql/socket.fail.fixture.ts

diff --git a/src/sql/postgres/PostgresSQLConnection.zig b/src/sql/postgres/PostgresSQLConnection.zig
index 8af36bbf2d..634c33aed0 100644
--- a/src/sql/postgres/PostgresSQLConnection.zig
+++ b/src/sql/postgres/PostgresSQLConnection.zig
@@ -717,16 +717,6 @@ pub fn call(globalObject: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JS
         },
     };
 
-    ptr.updateHasPendingActivity();
-    ptr.poll_ref.ref(vm);
-    const js_value = ptr.toJS(globalObject);
-    js_value.ensureStillAlive();
-    ptr.js_value = js_value;
-
-    js.onconnectSetCached(js_value, globalObject, on_connect);
-    js.oncloseSetCached(js_value, globalObject, on_close);
-    bun.analytics.Features.postgres_connections += 1;
-
     {
         const hostname = hostname_str.toUTF8(bun.default_allocator);
         defer hostname.deinit();
@@ -761,9 +751,18 @@ pub fn call(globalObject: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JS
                 },
             };
         }
-        ptr.resetConnectionTimeout();
     }
 
+    // only call toJS if connectUnixAnon does not fail immediately
+    ptr.updateHasPendingActivity();
+    ptr.resetConnectionTimeout();
+    ptr.poll_ref.ref(vm);
+    const js_value = ptr.toJS(globalObject);
+    js_value.ensureStillAlive();
+    ptr.js_value = js_value;
+    js.onconnectSetCached(js_value, globalObject, on_connect);
+    js.oncloseSetCached(js_value, globalObject, on_close);
+    bun.analytics.Features.postgres_connections += 1;
     return js_value;
 }
 
diff --git a/test/js/sql/socket.fail.fixture.ts b/test/js/sql/socket.fail.fixture.ts
new file mode 100644
index 0000000000..401e949fc6
--- /dev/null
+++ b/test/js/sql/socket.fail.fixture.ts
@@ -0,0 +1,19 @@
+setTimeout(() => {
+  // no need to wait we know at this point that the test passed
+  process.exit(0);
+}, 100);
+for (let i = 0; i < 3; i++) {
+  try {
+    const sql = new Bun.SQL({
+      url: "postgres://-invalid-:1234/postgres",
+      max: 1,
+      idleTimeout: 1,
+      connectionTimeout: 1,
+      maxLifetime: 1,
+    });
+    await sql.connect();
+  } catch {
+  } finally {
+    Bun.gc(true);
+  }
+}
diff --git a/test/js/sql/sql.test.ts b/test/js/sql/sql.test.ts
index d8475b0164..5ff1791a0a 100644
--- a/test/js/sql/sql.test.ts
+++ b/test/js/sql/sql.test.ts
@@ -1,6 +1,6 @@
 import { $, randomUUIDv7, sql, SQL } from "bun";
 import { afterAll, describe, expect, mock, test } from "bun:test";
-import { bunExe, isCI, isLinux, tempDirWithFiles } from "harness";
+import { bunEnv, bunExe, isCI, isLinux, tempDirWithFiles } from "harness";
 import path from "path";
 const postgres = (...args) => new sql(...args);
 
@@ -11096,3 +11096,16 @@ CREATE TABLE ${table_name} (
     });
   });
 }
+
+describe("should proper handle connection errors", () => {
+  test("should not crash if connection fails", async () => {
+    const result = Bun.spawnSync([bunExe(), path.join(import.meta.dirname, "socket.fail.fixture.ts")], {
+      cwd: import.meta.dir,
+      env: bunEnv,
+      stdin: "ignore",
+      stdout: "inherit",
+      stderr: "pipe",
+    });
+    expect(result.stderr?.toString()).toBeFalsy();
+  });
+});