PR #32803 - oven-sh/bun

What does this PR do?

Fixes a Bun.RedisClient connection-lifecycle bug where a stale auto-reconnect timer starts a second, competing connection and orphans the one already in flight. On debug/assertion builds the orphan's late traffic trips:

panic: assertion failed: self.this_value.get().is_strong()
  on_valkey_connect <- handle_hello_response <- handle_response <- ValkeyClient::on_data <- SocketHandler::on_data<false>

On release builds the same sequence silently runs socket callbacks against a client whose JS wrapper is only weakly referenced (it is also an fd leak: the orphaned socket is never closed).

Reproduction (scripted in-process RESP3 server; full version is the test):

A connected client loses its connection; the auto-reconnect branch arms a 50ms reconnect timer.
Before the timer fires, user code calls connect(). do_connect() starts a new socket; the timer stays armed.
The server withholds the new socket's HELLO reply, so is_reconnecting is still set when the timer fires. on_reconnect_timer() -> reconnect() -> connect() creates another socket and assigns it to client.socket, orphaning the in-flight one.
The orphan is live and un-owned. Its late on_open resets is_authenticated and writes a HELLO, but never sets status = Connecting or re-upgrades the wrapper ref, so by the time the reply arrives the client is Disconnected with a weak (or collected) wrapper, and on_valkey_connect asserts.

Cause

reconnect() only checked is_reconnecting. A reconnect timer armed by a connection loss is not cancelled by a later explicit connect(), and connect() assigns client.socket without closing the previous one, so the timer firing while a connection is in flight (Status::Connecting, or even Connected) clobbers a live socket. is_reconnecting is only cleared by a successful HELLO, so a withheld or slow HELLO response is enough to keep the stale timer armed and active.

Fix

reconnect() returns unless status == Disconnected. Disconnected is the only state with no live socket attached (every transition into it detaches or closes the socket first), so connect() can no longer overwrite a live one.
connect() now debug_assert!s that the previous socket is closed or detached, so any future path that violates the one-live-socket invariant fails loudly in debug and fuzz builds instead of orphaning a socket.

Not included here: making the SocketHandler callbacks ignore events from a socket other than client.socket (defense in depth for the same class). With the invariant restored at the source that condition is unreachable, and filtering on_close/on_connect_error interacts with the per-socket keep-alive refcount, so it deserves its own change if we want it.

Related but distinct open PRs in this area: #32768 and #32779 fix synchronously-failing connect() paths; this one is about a successful competing connect.

Verification

test/js/valkey/valkey-gc.test.ts gets a spawned fixture with a scripted in-process RESP3 server. It has two oracles, so it does not depend on debug assertions:

the server counts accepted connections: 3 with the fix (control, first connection, explicit reconnect), 4 on broken builds (the timer's competing socket);
the fixture must exit 0 (on unfixed debug builds it dies on the assertion above).

# unfixed (src/ stashed), bun bd test test/js/valkey/valkey-gc.test.ts
panic: assertion failed: self.this_value.get().is_strong()
(fail) explicit connect() while the auto-reconnect timer is armed does not orphan the in-flight socket

# unfixed release (USE_SYSTEM_BUN=1 bun test test/js/valkey/valkey-gc.test.ts)
error: attempt 0: expected 3 accepted connections, got 4
(fail) explicit connect() while the auto-reconnect timer is armed does not orphan the in-flight socket

# with the fix (bun bd test test/js/valkey/valkey-gc.test.ts)
6 pass, 0 fail

[review] gate passed · iteration 4 · 2 files touched

fails on main (without fix)

ASAN without fix: 1 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/valkey/valkey-gc.test.ts
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
bun test v1.4.0 (e7f52e674)

test/js/valkey/valkey-gc.test.ts:
(pass) custom setter with a foreign receiver throws instead of corrupting the heap [552.14ms]
(pass) RedisClient survives GC after a command throws during argument validation [617.77ms]
(pass) rejects a RESP simple-string reply whose line terminator never arrives [565.30ms]
(pass) RedisClient survives GC across many short-lived instances [725.10ms]
============================================================
Bun Debug v1.4.0 (e7f52e674) Linux x64
Linux Kernel v6.17.0 | glibc v2.41
CPU: sse42 popcnt avx avx2 avx512
Args: "/workspace/bun/build/debug/bun-debug" "-e" "\n    const ATTEMPTS = 3;\n\n    async function attempt() {\n      const helloHeld = Promise.withResolvers();\n      let srvFi
... (truncated)

release without fix: 1 FAILED
bun test v1.4.0-canary.1 (1498d7b77)

test/js/valkey/valkey-gc.test.ts:
(pass) RedisClient survives GC after a command throws during argument validation [15.80ms]
(pass) custom setter with a foreign receiver throws instead of corrupting the heap [14.44ms]
(pass) RedisClient survives GC across many short-lived instances [15.12ms]
(pass) rejects a RESP simple-string reply whose line terminator never arrives [18.35ms]
113 |       const accepted = await attempt();
114 |       // Exactly three: control, the first connection, the explicit reconnect.
115 |       // A fourth means a stale reconnect timer opened a competing socket and
116 |       // orphaned one of them.
117 |       if (accepted !== 3) {
118 |         throw new Error("attempt " + i + ": expected 3 accepted connections, got " + accepted);
                        ^
error: attempt 0: expected 3 accepted connections, got 4
      at /workspace/bun/[eval]:118:19

Bun v1.4.0-canary.1+1498d7b77 (Linux x64)
345 |     stderr: "inherit",
346 |   });
347 | 
348 |   const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]);
349 | 
350 |   expect(stdout.trim()).toBe("OK");
                           
... (truncated)

passes on PR (with fix)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/valkey/valkey-gc.test.ts
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
bun test v1.4.0 (e7f52e674)

test/js/valkey/valkey-gc.test.ts:
(pass) custom setter with a foreign receiver throws instead of corrupting the heap [517.24ms]
(pass) RedisClient survives GC after a command throws during argument validation [698.10ms]
(pass) rejects a RESP simple-string reply whose line terminator never arrives [656.70ms]
(pass) RedisClient survives GC across many short-lived instances [763.68ms]
(pass) explicit connect() while the auto-reconnect timer is armed does not orphan the in-flight socket [1598.13ms]
(pass) getBuffer replies survive GC with adopted backing stores intact [2197.56ms]

 6 pass
 0 fail
 17 expect() calls
Ran 6 tests across 1 file. [6.91s]
__F:0:S:0

release with fix: all passed
$ bun scripts/build.ts --profile=release
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     e7f52e674d
  features     (none)

22 deps, 106 codegen, 1168 objects in 831ms

ninja: Entering directory `/workspace/bun/build/release'
[1/1231] install /workspace/bun
bun install v1.4.0-canary.1 (1498d7b77)

Checked 124 installs across 170 packages (no changes) [16.00ms]
[2/1231] install /workspace/bun/packages/bun-error
bun install v1.4.0-canary.1 (1498d7b77)

Checked 1 install across 2 packages (no changes) [8.00ms]
[3/1231] fetch picohttpparser
[picohttpparser] up to date
[4/1231] install /workspace/bun/src/node-fallbacks
bun install v1.4.0-canary.1 (1498d7b77)

Checked 129 installs across 147 packages (no changes) [10.00ms]
[5/1231] gen bindgenv2
[6/1231] gen ErrorCode+*.h
[7/1231] fetch libjpeg-turbo
[libjpeg-turbo] up to date

... (truncated)

diff hotspot

src/runtime/valkey_jsc/js_valkey.rs |  11 +++
 test/js/valkey/valkey-gc.test.ts    | 143 ++++++++++++++++++++++++++++++++++++
 2 files changed, 154 insertions(+)

gate history · 1 passed · 0 rejected · iteration 4

evidence per changed file

file                                 reads  edits  tests
src/runtime/valkey_jsc/js_valkey.rs     13      3     15
test/js/valkey/valkey-gc.test.ts         4      3     15

#32803 valkey: don't let a stale reconnect timer clobber an in-flight socket

What does this PR do?

Cause

Fix

Verification

valkey: don't let a stale reconnect timer clobber an in-flight socket