-
Notifications
You must be signed in to change notification settings - Fork 14.8k
[lldb] Protect the selected frame idx in StackFrameList #150718
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesProtected m_selected_frame_idx using the StackFrameList mutex. All other readers and writes already take the m_list_mutex so we can use that to protect concurrent access. Full diff: https://github.com/llvm/llvm-project/pull/150718.diff 1 Files Affected:
diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp
index 16cd2548c2784..4c5f41f292e56 100644
--- a/lldb/source/Target/StackFrameList.cpp
+++ b/lldb/source/Target/StackFrameList.cpp
@@ -783,6 +783,8 @@ void StackFrameList::SelectMostRelevantFrame() {
uint32_t
StackFrameList::GetSelectedFrameIndex(SelectMostRelevant select_most_relevant) {
+ std::shared_lock<std::shared_mutex> guard(m_list_mutex);
+
if (!m_selected_frame_idx && select_most_relevant)
SelectMostRelevantFrame();
if (!m_selected_frame_idx) {
|
Speculative fix for the crashes I'm seeing on the Linux bots in #148994. Both backtraces show the default event handler thread in |
This means that we are holding the stack frame list mutex over the call to SelectMostRelevantFrame. But SelectMostRelevantFrame can call into arbitrary Python - since that's where the FrameRecognizers are triggered. These mutexes aren't recursive so if the Frame Recognizer does anything that needs the frame mutex, this is going to deadlock. |
Protected m_selected_frame_idx using a mutex.
0c77d94
to
059b241
Compare
Yeah, I came to the same conclusion. I ended going with a separate, recursive mutex. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/20409 Here is the relevant piece of the build log for the reference
|
Protected m_selected_frame_idx with a mutex. To avoid deadlocks, always acquire m_selected_frame_mutex after m_list_mutex. I'm using a recursive mutex because GetSelectedFrameIndex may indirectly call SetSelectedFrame.
Protected m_selected_frame_idx using the StackFrameList mutex. All other readers and writes already take the m_list_mutex so we can use that to protect concurrent access.