Fix self-join when a dialed WebSocket is released on its own io thread - #232
Merged
Merged
Conversation
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.
|
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes the main-branch CI failure in
bazel consumer (ubuntu-24.04)(run):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.
AnAwaitedReceiveDeadlineTicksWithoutEndingTheSessionhands a copy of its dialed socket to aDetachedcoroutine, and the coroutine finishes on that socket's io thread. When the test body releasesdialedfirst, the coroutine frame drops the last reference, so~DialedWebSocketruns on its ownrunner_thread and callsrunner_.join(). That self-join throws inside anoexceptdestructor. The process dies a moment later, while the next test is running. Nothing about this is specific to #230 or #231: anyDetachedloop that owns its dialed socket can hit it.Fix: the io thread now co-owns the connection and session through a shared
DialedIo.run()returns. Theio.stop()in the destructor makesrun()return immediately.Testing
BeastWebSocketTest.ADetachedLoopMayReleaseTheLastHandleOnTheSocketsOwnIoThread: aDetachedloop 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 sameResource deadlock avoided. After the fix it passed 50 of 50 repeated runs.beast_websocket_test,beast_transport_testandasync_event_stream_testpass with--config=ci.beast_websocket_test, 10 runs, are clean.async_acceptance_testpasses 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
bazel test //...and(cd codegen && gradle build spotlessCheck)pass locally. Partly: I ran the affected suites; codegen is untouched.