Fix crash from joining the event thread on itself#1047
Open
Conversation
plafosse
reviewed
Apr 6, 2026
ui/ui.cpp
Outdated
| frame->navigate(m_controller->GetData(), address, true, true); | ||
| } | ||
|
|
||
| openDebuggerSideBar(frame); |
Member
There was a problem hiding this comment.
this is the same change as the other PR?
Member
Author
There was a problem hiding this comment.
This is a change introduced earlier: #1032. The reason that I wanted it to no longer focus the debugger sidebar is that if the user is using sidekick to automate the debugger, this will cause the sidekick sidebar to lose focus.
I think I made some git mistake so that it gets wiped out unexpectedly. I will add it back
When DebuggerController::EventHandler handles TargetExitedEventType, DetachedEventType, or LaunchFailureEventType, it deletes m_accessor. The DebuggerFileAccessor holds the last DbgRef<DebuggerController>, so deleting it triggers ~DebuggerController, which calls m_debuggerEventThread.join(). Since we're already on that thread, join() throws std::system_error(resource_deadlock_would_occur), causing an abort. Defer the accessor deletion to a worker thread so that if it triggers controller destruction, the destructor runs off the event thread and join() succeeds normally. Fixes #1038 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
BinaryNinja's worker threads are meant for analysis tasks. Use a plain detached thread instead for deferring the m_accessor deletion off the event thread. Also expanded the comment to document the race condition root cause: Destroy() can remove the global array ref between SetConnectionStatus (NotConnected) and the accessor deletion, making the accessor's DbgRef the last reference to the controller. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
xusheng6
force-pushed
the
test_1038-crash-join-event-thread
branch
from
bf18d73 to
d490308
Compare
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
2 participants
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.
Summary
delete m_accessorinEventHandlerto a detachedstd::threadDebuggerFileAccessorholds aDbgRef<DebuggerController>, and when it's the last reference, deleting it on the event thread triggers~DebuggerControllerwhich callsm_debuggerEventThread.join()— crashing because we ARE the event threadEventHandlersetsConnectionStatus(NotConnected)before deleting the accessor, and another thread (UI close path or Python) can observe this, callDestroy()to remove the global array ref, making the accessor'sDbgRefthe last referencejoin()succeeds normallyTest plan
Fixes #1038
🤖 Generated with Claude Code