Skip to content

Fix SIGSEGV at process exit after opening a WebSocket - #30

Open
tommedema wants to merge 1 commit into
lexiforest:mainfrom
tommedema:fix/ws-connect-segfault
Open

Fix SIGSEGV at process exit after opening a WebSocket#30
tommedema wants to merge 1 commit into
lexiforest:mainfrom
tommedema:fix/ws-connect-segfault

Conversation

@tommedema

Copy link
Copy Markdown

Fixes #29.

The bug

Opening a WebSocket makes the process die with SIGSEGV during teardown — after the script has run to completion and produced all of its output, so it looks like a clean run until you check $?.

$ node -e 'import("impers").then(async ({wsConnect}) => { const ws = await wsConnect("wss://echo.websocket.org"); await ws.close(); console.log("done") })'
done
$ echo $?
139

Root cause

performConnect ran curl_easy_perform through Koffi's .async(), which dispatches to a libuv worker thread. Driving libcurl from one leaves per-thread state behind whose pthread TSD destructor lives in the libcurl image. At process exit the worker thread is torn down, _pthread_tsd_cleanup calls that destructor after the image has gone, and the process faults:

exception:   EXC_BAD_ACCESS (SIGSEGV), KERN_INVALID_ADDRESS
  ???                       <- unmapped
  _pthread_tsd_cleanup
  _pthread_exit
  _pthread_start
  thread_start

It is not WebSocket-specific: a plain HTTPS GET issued through .async() faults identically. WebSockets were simply the only code path that used it, which is why normal HTTP usage is stable.

The fix

Making the connect synchronous removes the crash, but it deadlocks any caller whose server shares the process — which is exactly what this repository's tests do, and the reason .async() was chosen in the first place. Both constraints have to hold at once.

The multi interface satisfies both. curl_multi_perform returns as soon as there is nothing to do right now, so the handshake advances across event-loop turns without ever leaving the main thread — no worker thread, and no blocking. The bindings were already present in src/ffi/libcurl.ts.

The easy handle stays attached to the multi for the life of the socket; removing it would drop the connection that CONNECT_ONLY exists to keep. curl_ws_send / curl_ws_recv already ran synchronously on the main thread, so the handshake is now consistent with the rest of the class.

curl_easy_perform_async had no other caller and was never exported from the package entry, so it is removed rather than left as a trap for the next caller.

Verification

  • The regression test spawns a child process on purpose: nothing observable from inside the process can catch a fault that happens after the last line runs, only the exit status can. It covers both closing the socket and leaving it open. It fails on the unfixed code and passes on the fix (verified by rebuilding from a stash).
  • It builds dist/ itself in beforeAll rather than assuming it is present — a stale build would let the test pass against the very code it exists to reject.
  • The WebSocket suite skipped in fbd0ac9 is re-enabled and passes.
  • Full suite: 156 passed, 1 suite skipped (the live-fingerprint tests, which need IMPERSONATE_API_KEY).
  • npm run lint reports the same 29 pre-existing errors as main — none added.
  • Verified live against wss://echo.websocket.org: connect, send, recv round-trip, close, exit status 0.

Tested on darwin/arm64, Node v25.8.2, macOS 26.5.2.

Opening a WebSocket made the process die with SIGSEGV during teardown, after
the script had already run to completion and produced all of its output.

`performConnect` ran `curl_easy_perform` through Koffi's `.async()`, which
dispatches to a libuv worker thread. Driving libcurl from one leaves per-thread
state behind whose pthread TSD destructor lives in the libcurl image. At exit
the worker thread is torn down, `_pthread_tsd_cleanup` calls that destructor
after the image has gone, and the process faults:

    EXC_BAD_ACCESS (SIGSEGV), KERN_INVALID_ADDRESS
    ???  ->  _pthread_tsd_cleanup  ->  _pthread_exit  ->  _pthread_start

It is not WebSocket-specific: a plain HTTPS GET issued through `.async()`
faults the same way. WebSockets are simply the only path that used it.

Making the connect synchronous fixes the crash but deadlocks any caller whose
server shares the process — which is what the tests in this repository do, and
why `.async()` was chosen. So the handshake is driven with the multi interface
instead: `curl_multi_perform` returns as soon as there is nothing to do right
now, so the connect advances across event-loop turns without ever leaving the
main thread. Both constraints hold at once. The handle stays attached to the
multi for the life of the socket, since removing it would drop the connection
that CONNECT_ONLY exists to keep.

`curl_easy_perform_async` had no other caller and was never exported from the
package entry, so it is removed rather than left as a trap.

The regression test spawns a child process, because nothing observable from
inside the process can catch a fault that happens after the last line runs —
only the exit status can. It fails on the unfixed code and passes on the fix.

With the crash gone, the WebSocket suite skipped in fbd0ac9 is re-enabled; it
passes, along with the rest of the suite.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wsConnect: process exits with SIGSEGV in _pthread_tsd_cleanup

1 participant