Skip to content

Fix self-join when a dialed WebSocket is released on its own io thread - #232

Merged
aaylward merged 1 commit into
mainfrom
claude/optimistic-pasteur-m6aq2e
Sep 24, 2026
Merged

aaylward merged 1 commit into
mainfrom
claude/optimistic-pasteur-m6aq2e

Conversation

@aaylward

@aaylward aaylward commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

What

Fixes the main-branch CI failure in bazel consumer (ubuntu-24.04) (run):

[ RUN ] AsyncAcceptanceTest.ThreeSessionsShareOneHandlerThreadAndAFanOutRegistry
terminate called after throwing an instance of 'std::system_error'
  what():  Resource deadlock avoided

This is not a flaky test; it is a runtime bug that shows up intermittently. The crash comes from the test before the one it was reported in. AnAwaitedReceiveDeadlineTicksWithoutEndingTheSession hands a copy of its dialed socket to a Detached coroutine, and the coroutine finishes on that socket's io thread. When the test body releases dialed first, the coroutine frame drops the last reference, so ~DialedWebSocket runs on its own runner_ thread and calls runner_.join(). That self-join throws inside a noexcept destructor. The process dies a moment later, while the next test is running. Nothing about this is specific to #230 or #231: any Detached loop that owns its dialed socket can hit it.

Fix: the io thread now co-owns the connection and session through a shared DialedIo.

  • If the destructor runs on the io thread, it detaches instead of joining. The thread's own reference frees the session and then the connection, the same order as before, after run() returns. The io.stop() in the destructor makes run() return immediately.
  • If the destructor runs anywhere else, it joins as before.

Testing

  • New BeastWebSocketTest.ADetachedLoopMayReleaseTheLastHandleOnTheSocketsOwnIoThread: a Detached loop holds the only reference to a dialed socket, is resumed by a server push, and finishes on the io thread. Before the fix it failed every run with the same Resource deadlock avoided. After the fix it passed 50 of 50 repeated runs.
  • beast_websocket_test, beast_transport_test and async_event_stream_test pass with --config=ci.
  • TSan (gcc) on the Beast suites is clean.
  • ASan and LSan (gcc) on beast_websocket_test, 10 runs, are clean.
  • The consumer module's async_acceptance_test passes 200 of 200 runs locally. It didn't reproduce locally before the fix either (300 runs); the new runtime test is the one that reproduces it reliably.

CI's clang ASan/UBSan and clang TSan will run on this PR; the local clang has no sanitizer runtimes.

Checklist

  • Tests added/updated for the change
  • bazel test //... and (cd codegen && gradle build spotlessCheck) pass locally. Partly: I ran the affected suites; codegen is untouched.
  • Formatting clean (clang-format, buildifier, spotless)
  • Architectural decisions recorded as an ADR (if applicable). Not applicable: this is a bug fix.

When the last handle to a BeastWebSocketClient::Dial socket dropped
inside one of its own completions, ~DialedWebSocket ran on the socket's
io thread and joined that same thread, so std::terminate fired ("Resource
deadlock avoided"). A Detached loop that owns its socket and finishes on
the io thread does exactly this whenever it outlives the caller's
reference. That is the intermittent main-branch failure in the consumer
module's async_acceptance_test: the #130 watchdog test's coroutine was
sometimes the last owner, and the crash landed during the next test.

The io thread now co-owns the connection and session (DialedIo). A
destructor running on that thread detaches it, and the thread's own
reference frees them after run() returns, in the same order as before
(session, then connection). Destruction elsewhere still joins.

Regression test: a Detached loop owns the only reference to a dialed
socket and finishes on its io thread. Before this fix it terminated with
the same error every time.
@cursor

cursor Bot commented Sep 24, 2026

Copy link
Copy Markdown

Reviewed the ownership and teardown paths, including destruction on the runner thread, destruction from another thread, session/connection member order, work-guard shutdown, and callback lifetime. The runner’s independent DialedIo ownership closes the self-join/UAF window, and the regression test exercises the last-handle-on-completion path. I found no correctness issues.

@aaylward
aaylward merged commit 7fe4d8d into main Sep 24, 2026
16 checks passed
@aaylward
aaylward deleted the claude/optimistic-pasteur-m6aq2e branch September 24, 2026 21:00
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.

2 participants