From 5718818b3df9c710ea27c2f3ccff36d0ab49e7f2 Mon Sep 17 00:00:00 2001
From: Claude Bot <claude-bot@bun.sh>
Date: Sun, 26 Oct 2025 05:12:08 +0000
Subject: [PATCH] Address review comments: fix shared_context cleanup and
 rewrite error test
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Fix comment about .deinit = true in server.zig to clarify it enables automatic cleanup
- Fix shared_context cleanup in ServerWebSocket.onClose() to avoid use-after-free
- Rewrite WebSocket error handler test to use public API instead of accessing ws._socket
- Remove detailed error assertions since error parameter may be undefined

All 17 tests passing.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
---
 src/bun.js/api/server.zig                     |  4 ++++
 src/bun.js/api/server/ServerWebSocket.zig     |  6 +++--
 .../js/bun/http/serve-route-websocket.test.ts | 23 ++++++++++---------
 3 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/src/bun.js/api/server.zig b/src/bun.js/api/server.zig
index 520feb71d0..e6dd5e87c2 100644
--- a/src/bun.js/api/server.zig
+++ b/src/bun.js/api/server.zig
@@ -563,12 +563,16 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d
 
         /// Per-route WebSocket contexts. Index is (id - 2) where id comes from app.ws()
         /// Use Shared pointers to ensure stable addresses (ServerWebSocket stores raw pointers to handlers)
+        /// When ref count reaches 0, WebSocketServerContext.deinit() is automatically called to unprotect JSValues
         route_websocket_contexts: std.ArrayListUnmanaged(SharedWebSocketContext) = .{},
 
         on_clienterror: jsc.Strong.Optional = .empty,
 
         inspector_server_id: jsc.Debugger.DebuggerId = .init(0),
 
+        /// Shared pointer type for route-specific WebSocket contexts
+        /// .deinit = true enables automatic cleanup: when the last reference is released,
+        /// WebSocketServerContext.deinit() is called to unprotect JSValues
         pub const SharedWebSocketContext = bun.ptr.shared.WithOptions(*WebSocketServerContext, .{ .deinit = true });
         pub const doStop = host_fn.wrapInstanceMethod(ThisServer, "stopFromJS", false);
         pub const dispose = host_fn.wrapInstanceMethod(ThisServer, "disposeFromJS", false);
diff --git a/src/bun.js/api/server/ServerWebSocket.zig b/src/bun.js/api/server/ServerWebSocket.zig
index b2b6eacae0..b4eae19112 100644
--- a/src/bun.js/api/server/ServerWebSocket.zig
+++ b/src/bun.js/api/server/ServerWebSocket.zig
@@ -329,8 +329,10 @@ pub fn onClose(this: *ServerWebSocket, _: uws.AnyWebSocket, code: i32, message:
         // Release the shared context reference if we have one
         // When the last reference is released, WebSocketServerContext.deinit()
         // will be called automatically to unprotect JSValues
-        if (this.#shared_context) |*shared_ctx| {
-            shared_ctx.deinit();
+        if (this.#shared_context) |shared_ctx| {
+            var ctx = shared_ctx;
+            this.#shared_context = null;
+            ctx.deinit();
         }
     }
 
diff --git a/test/js/bun/http/serve-route-websocket.test.ts b/test/js/bun/http/serve-route-websocket.test.ts
index 44103703e9..39787cd717 100644
--- a/test/js/bun/http/serve-route-websocket.test.ts
+++ b/test/js/bun/http/serve-route-websocket.test.ts
@@ -690,21 +690,22 @@ describe("Bun.serve() route-specific WebSocket handlers", () => {
     await new Promise(resolve => setTimeout(resolve, 100));
   });
 
-  test("websocket error handler is called", async () => {
+  test("websocket error handler is called on server-side exceptions", async () => {
     let errorCalled = false;
-    let errorMessage = "";
 
     using server = Bun.serve({
       port: 0,
       routes: {
         "/ws": {
           websocket: {
-            open(ws) {
-              ws.send("ready");
+            message(ws, message) {
+              // Trigger an error when receiving "trigger-error"
+              if (message === "trigger-error") {
+                throw new Error("Intentional test error");
+              }
             },
             error(ws, error) {
               errorCalled = true;
-              errorMessage = error?.message || "error occurred";
             },
           },
           upgrade(req, server) {
@@ -717,14 +718,14 @@ describe("Bun.serve() route-specific WebSocket handlers", () => {
     const ws = new WebSocket(`ws://localhost:${server.port}/ws`);
     await new Promise(resolve => (ws.onopen = resolve));
 
-    // Send invalid data to trigger error
-    // @ts-ignore - accessing private property for testing
-    ws._socket?.write(Buffer.from([0xff, 0xff, 0xff, 0xff]));
+    // Send message that triggers server-side error
+    ws.send("trigger-error");
 
-    await new Promise(resolve => setTimeout(resolve, 200));
+    // Wait for error handler to be called
+    await Bun.sleep(100);
+
+    expect(errorCalled).toBe(true);
 
-    // Error handler may or may not be called depending on how WebSocket handles it
-    // Just verify the connection can be closed
     ws.close();
   });