diff --git a/src/bun.js/bindings/webcore/WebSocket.cpp b/src/bun.js/bindings/webcore/WebSocket.cpp index 4a3bb45ff25..778e3343cc8 100644 --- a/src/bun.js/bindings/webcore/WebSocket.cpp +++ b/src/bun.js/bindings/webcore/WebSocket.cpp @@ -920,6 +920,58 @@ void WebSocket::sendWebSocketString(const String& message, const Opcode op) updateHasPendingActivity(); } +// Called from close()/terminate() while m_state == CONNECTING. +// +// The Zig-side upgrade client's cancel() clears its back-pointer to us +// without calling didAbruptClose, so none of didConnect / +// didFailWithErrorCode / didClose will ever fire for this socket. We +// must therefore finish the close ourselves: cancel the upgrade, queue +// a task that moves to CLOSED, fires error + close events (spec "fail +// the WebSocket connection" path — code 1006, wasClean false), and +// releases the pending-activity ref taken in connect(). Without this +// the WebSocket stays in CLOSING with m_pendingActivityCount > 0 +// forever and is never garbage-collected. +void WebSocket::failConnectingWebSocket() +{ + ASSERT(m_state == CONNECTING); + m_state = CLOSING; + + if (m_upgradeClient != nullptr) { + void* upgradeClient = m_upgradeClient; + m_upgradeClient = nullptr; + bool useTLSClient = (m_connectionType == ConnectionType::TLS || m_connectionType == ConnectionType::ProxyTLS); + if (useTLSClient) { + Bun__WebSocketHTTPSClient__cancel(upgradeClient); + } else { + Bun__WebSocketHTTPClient__cancel(upgradeClient); + } + } + + if (auto* context = scriptExecutionContext()) { + context->postTask([protectedThis = Ref { *this }](ScriptExecutionContext& context) { + if (protectedThis->m_state == CLOSED) + return; + protectedThis->m_state = CLOSED; + if (protectedThis->m_native.onClose) { + protectedThis->m_native.onClose(protectedThis->m_native.ctx, 1006); + } else { + // Spec: close() while CONNECTING runs "fail the WebSocket + // connection", which requires an error event before the + // close event. Matches Chrome/Firefox and npm ws. + auto reason = "WebSocket is closed before the connection is established"_s; + auto eventInit = createErrorEventInit(protectedThis, reason, context.jsGlobalObject()); + protectedThis->dispatchEvent(ErrorEvent::create(eventNames().errorEvent, WTF::move(eventInit), EventIsTrusted::Yes)); + protectedThis->dispatchEvent(CloseEvent::create(false, 1006, reason)); + } + protectedThis->disablePendingActivity(); + }); + } else { + m_state = CLOSED; + disablePendingActivity(); + } + updateHasPendingActivity(); +} + ExceptionOr WebSocket::close(std::optional optionalCode, const String& reason) { int code = optionalCode ? optionalCode.value() : static_cast(1000); @@ -938,18 +990,7 @@ ExceptionOr WebSocket::close(std::optional optionalCode, c if (m_state == CLOSING || m_state == CLOSED) return {}; if (m_state == CONNECTING) { - m_state = CLOSING; - if (m_upgradeClient != nullptr) { - void* upgradeClient = m_upgradeClient; - m_upgradeClient = nullptr; - bool useTLSClient = (m_connectionType == ConnectionType::TLS || m_connectionType == ConnectionType::ProxyTLS); - if (useTLSClient) { - Bun__WebSocketHTTPSClient__cancel(upgradeClient); - } else { - Bun__WebSocketHTTPClient__cancel(upgradeClient); - } - } - updateHasPendingActivity(); + failConnectingWebSocket(); return {}; } m_state = CLOSING; @@ -994,18 +1035,7 @@ ExceptionOr WebSocket::terminate() if (m_state == CLOSING || m_state == CLOSED) return {}; if (m_state == CONNECTING) { - m_state = CLOSING; - if (m_upgradeClient != nullptr) { - void* upgradeClient = m_upgradeClient; - m_upgradeClient = nullptr; - bool useTLSClient = (m_connectionType == ConnectionType::TLS || m_connectionType == ConnectionType::ProxyTLS); - if (useTLSClient) { - Bun__WebSocketHTTPSClient__cancel(upgradeClient); - } else { - Bun__WebSocketHTTPClient__cancel(upgradeClient); - } - } - updateHasPendingActivity(); + failConnectingWebSocket(); return {}; } m_state = CLOSING; diff --git a/src/bun.js/bindings/webcore/WebSocket.h b/src/bun.js/bindings/webcore/WebSocket.h index 9d9077685a9..f9cde8b5396 100644 --- a/src/bun.js/bindings/webcore/WebSocket.h +++ b/src/bun.js/bindings/webcore/WebSocket.h @@ -271,6 +271,7 @@ class WebSocket final : public RefCounted, public EventTargetWithInli void didReceiveClose(CleanStatus wasClean, unsigned short code, WTF::String reason, bool isConnectionError = false); void didUpdateBufferedAmount(unsigned bufferedAmount); void didStartClosingHandshake(); + void failConnectingWebSocket(); void sendWebSocketString(const String& message, const Opcode opcode); void sendWebSocketData(const char* data, size_t length, const Opcode opcode); diff --git a/test/js/web/websocket/websocket-close-connecting.test.ts b/test/js/web/websocket/websocket-close-connecting.test.ts new file mode 100644 index 00000000000..f4dc7dfcf62 --- /dev/null +++ b/test/js/web/websocket/websocket-close-connecting.test.ts @@ -0,0 +1,90 @@ +// close()/terminate() during CONNECTING must transition to CLOSED, fire +// error + close events, and release the pending-activity ref so the +// WebSocket can be garbage-collected. Previously the Zig upgrade client +// was cancelled without any completion callback reaching C++, leaving +// the socket stuck in CLOSING with hasPendingActivity() permanently true +// — never GC'd, close event never fired, process never exited. + +import { heapStats } from "bun:jsc"; +import { describe, expect, it } from "bun:test"; +import { createServer, type AddressInfo } from "net"; + +describe.each(["close", "terminate"] as const)("%s() during CONNECTING", method => { + it("fires error + close events and transitions to CLOSED", async () => { + // TCP server that accepts connections but never responds — keeps the + // WebSocket in CONNECTING until we abort it. + const listening = Promise.withResolvers(); + const server = createServer(() => {}).listen(0, "127.0.0.1", listening.resolve); + await listening.promise; + try { + const port = (server.address() as AddressInfo).port; + const ws = new WebSocket(`ws://127.0.0.1:${port}`); + expect(ws.readyState).toBe(WebSocket.CONNECTING); + + const events: { type: string; message?: string }[] = []; + ws.onerror = e => events.push({ type: e.type, message: (e as ErrorEvent).message }); + const closed = new Promise(resolve => { + ws.onclose = e => { + events.push({ type: e.type }); + resolve(e); + }; + }); + + ws[method](); + expect(ws.readyState).toBe(WebSocket.CLOSING); + + const ev = await closed; + // Spec "fail the WebSocket connection": error event then close event. + expect(events.map(e => e.type)).toEqual(["error", "close"]); + expect(events[0].message).toContain("closed before the connection is established"); + expect({ + readyState: ws.readyState, + code: ev.code, + wasClean: ev.wasClean, + }).toEqual({ + readyState: WebSocket.CLOSED, + code: 1006, + wasClean: false, + }); + } finally { + server.close(); + } + }); + + it("does not leak", async () => { + function getWebSocketCount() { + Bun.gc(true); + return heapStats().objectTypeCounts?.WebSocket || 0; + } + + const listening = Promise.withResolvers(); + const server = createServer(() => {}).listen(0, "127.0.0.1", listening.resolve); + await listening.promise; + try { + const port = (server.address() as AddressInfo).port; + const before = getWebSocketCount(); + + const closes: Promise[] = []; + for (let i = 0; i < 64; i++) { + const ws = new WebSocket(`ws://127.0.0.1:${port}`); + closes.push(new Promise(resolve => (ws.onclose = resolve))); + ws[method](); + } + await Promise.all(closes); + + // disablePendingActivity is posted as a separate task after the close + // event fires; drain those plus any pending socket-close callbacks + // before counting. Without the fix, all 64 instances remain alive. + let after: number; + for (let i = 0; i < 10; i++) { + await new Promise(resolve => setImmediate(resolve)); + after = getWebSocketCount(); + if (after - before <= 5) break; + } + + expect(after! - before).toBeLessThanOrEqual(5); + } finally { + server.close(); + } + }); +});