Skip to content

Commit 7fb620a

Browse files
authored
Fix a deadlock in ModuleList when starting a standalone lldb client/server (#148774)
Summary: There was a deadlock was introduced by [PR #146441](#146441) which changed `CurrentThreadIsPrivateStateThread()` to `CurrentThreadPosesAsPrivateStateThread()`. This change caused the execution path in [`ExecutionContextRef::SetTargetPtr()`](https://github.com/llvm/llvm-project/blob/10b5558b61baab59c7d3dff37ffdf0861c0cc67a/lldb/source/Target/ExecutionContext.cpp#L513) to now enter a code block that was previously skipped, triggering [`GetSelectedFrame()`](https://github.com/llvm/llvm-project/blob/10b5558b61baab59c7d3dff37ffdf0861c0cc67a/lldb/source/Target/ExecutionContext.cpp#L522) which leads to a deadlock. Thread 1 gets m_modules_mutex in [`ModuleList::AppendImpl`](https://github.com/llvm/llvm-project/blob/96148f92146e5211685246722664e51ec730e7ba/lldb/source/Core/ModuleList.cpp#L218), Thread 3 gets m_language_runtimes_mutex in [`GetLanguageRuntime`](https://github.com/llvm/llvm-project/blob/96148f92146e5211685246722664e51ec730e7ba/lldb/source/Target/Process.cpp#L1501), but then Thread 1 waits for m_language_runtimes_mutex in [`GetLanguageRuntime`](https://github.com/llvm/llvm-project/blob/96148f92146e5211685246722664e51ec730e7ba/lldb/source/Target/Process.cpp#L1501) while Thread 3 waits for m_modules_mutex in [`ScanForGNUstepObjCLibraryCandidate`](https://github.com/llvm/llvm-project/blob/96148f92146e5211685246722664e51ec730e7ba/lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp#L57). This fixes the deadlock by adding a scoped block around the mutex lock before the call to the notifier, and moved the notifier call outside of the mutex-guarded section. The notifier call [`NotifyModuleAdded`](https://github.com/llvm/llvm-project/blob/96148f92146e5211685246722664e51ec730e7ba/lldb/source/Target/Target.cpp#L1810) should be thread-safe, since the module should be added to the `ModuleList` before the mutex is released, and the notifier doesn't modify the module list further, and the call is operates on local state and the `Target` instance. ### Deadlocked Thread backtraces: ``` * thread #3, name = 'dbg.evt-handler', stop reason = signal SIGSTOP * frame #0: 0x00007f2f1e2973dc libc.so.6`futex_wait(private=0, expected=2, futex_word=0x0000563786bd5f40) at futex-internal.h:146:13 /*... a bunch of mutex related bt ... */ liblldb.so.21.0git`std::lock_guard<std::recursive_mutex>::lock_guard(this=0x00007f2f0f1927b0, __m=0x0000563786bd5f40) at std_mutex.h:229:19 frame #8: 0x00007f2f27946eb7 liblldb.so.21.0git`ScanForGNUstepObjCLibraryCandidate(modules=0x0000563786bd5f28, TT=0x0000563786bd5eb8) at GNUstepObjCRuntime.cpp:60:41 frame #9: 0x00007f2f27946c80 liblldb.so.21.0git`lldb_private::GNUstepObjCRuntime::CreateInstance(process=0x0000563785e1d360, language=eLanguageTypeObjC) at GNUstepObjCRuntime.cpp:87:8 frame #10: 0x00007f2f2746fca5 liblldb.so.21.0git`lldb_private::LanguageRuntime::FindPlugin(process=0x0000563785e1d360, language=eLanguageTypeObjC) at LanguageRuntime.cpp:210:36 frame #11: 0x00007f2f2742c9e3 liblldb.so.21.0git`lldb_private::Process::GetLanguageRuntime(this=0x0000563785e1d360, language=eLanguageTypeObjC) at Process.cpp:1516:9 ... frame #21: 0x00007f2f2750b5cc liblldb.so.21.0git`lldb_private::Thread::GetSelectedFrame(this=0x0000563785e064d0, select_most_relevant=DoNoSelectMostRelevantFrame) at Thread.cpp:274:48 frame #22: 0x00007f2f273f9957 liblldb.so.21.0git`lldb_private::ExecutionContextRef::SetTargetPtr(this=0x00007f2f0f193778, target=0x0000563786bd5be0, adopt_selected=true) at ExecutionContext.cpp:525:32 frame #23: 0x00007f2f273f9714 liblldb.so.21.0git`lldb_private::ExecutionContextRef::ExecutionContextRef(this=0x00007f2f0f193778, target=0x0000563786bd5be0, adopt_selected=true) at ExecutionContext.cpp:413:3 frame #24: 0x00007f2f270e80af liblldb.so.21.0git`lldb_private::Debugger::GetSelectedExecutionContext(this=0x0000563785d83bc0) at Debugger.cpp:1225:23 frame #25: 0x00007f2f271bb7fd liblldb.so.21.0git`lldb_private::Statusline::Redraw(this=0x0000563785d83f30, update=true) at Statusline.cpp:136:41 ... * thread #1, name = 'lldb', stop reason = signal SIGSTOP * frame #0: 0x00007f2f1e2973dc libc.so.6`futex_wait(private=0, expected=2, futex_word=0x0000563785e1dd98) at futex-internal.h:146:13 /*... a bunch of mutex related bt ... */ liblldb.so.21.0git`std::lock_guard<std::recursive_mutex>::lock_guard(this=0x00007ffe62be0488, __m=0x0000563785e1dd98) at std_mutex.h:229:19 frame #8: 0x00007f2f2742c8d1 liblldb.so.21.0git`lldb_private::Process::GetLanguageRuntime(this=0x0000563785e1d360, language=eLanguageTypeC_plus_plus) at Process.cpp:1510:41 frame #9: 0x00007f2f2743c46f liblldb.so.21.0git`lldb_private::Process::ModulesDidLoad(this=0x0000563785e1d360, module_list=0x00007ffe62be06a0) at Process.cpp:6082:36 ... frame #13: 0x00007f2f2715cf03 liblldb.so.21.0git`lldb_private::ModuleList::AppendImpl(this=0x0000563786bd5f28, module_sp=ptr = 0x563785cec560, use_notifier=true) at ModuleList.cpp:246:19 frame #14: 0x00007f2f2715cf4c liblldb.so.21.0git`lldb_private::ModuleList::Append(this=0x0000563786bd5f28, module_sp=ptr = 0x563785cec560, notify=true) at ModuleList.cpp:251:3 ... frame #19: 0x00007f2f274349b3 liblldb.so.21.0git`lldb_private::Process::ConnectRemote(this=0x0000563785e1d360, remote_url=(Data = "connect://localhost:1234", Length = 24)) at Process.cpp:3250:9 frame #20: 0x00007f2f27411e0e liblldb.so.21.0git`lldb_private::Platform::DoConnectProcess(this=0x0000563785c59990, connect_url=(Data = "connect://localhost:1234", Length = 24), plugin_name=(Data = "gdb-remote", Length = 10), debugger=0x0000563785d83bc0, stream=0x00007ffe62be3128, target=0x0000563786bd5be0, error=0x00007ffe62be1ca0) at Platform.cpp:1926:23 ``` ## Test Plan: Built a hello world a.out Run server in one terminal: ``` ~/llvm/build/Debug/bin/lldb-server g :1234 a.out ``` Run client in another terminal ``` ~/llvm/build/Debug/bin/lldb -o "gdb-remote 1234" -o "b hello.cc:3" ``` Before: Client hangs indefinitely ``` ~/llvm/build/Debug/bin/lldb -o "gdb-remote 1234" -o "b main" (lldb) gdb-remote 1234 ^C^C ``` After: ``` ~/llvm/build/Debug/bin/lldb -o "gdb-remote 1234" -o "b hello.cc:3" (lldb) gdb-remote 1234 Process 837068 stopped * thread #1, name = 'a.out', stop reason = signal SIGSTOP frame #0: 0x00007ffff7fe4a60 ld-linux-x86-64.so.2`_start: -> 0x7ffff7fe4a60 <+0>: movq %rsp, %rdi 0x7ffff7fe4a63 <+3>: callq 0x7ffff7fe5780 ; _dl_start at rtld.c:522:1 ld-linux-x86-64.so.2`_dl_start_user: 0x7ffff7fe4a68 <+0>: movq %rax, %r12 0x7ffff7fe4a6b <+3>: movl 0x18067(%rip), %eax ; _dl_skip_args (lldb) b hello.cc:3 Breakpoint 1: where = a.out`main + 15 at hello.cc:4:13, address = 0x00005555555551bf (lldb) c Process 837068 resuming Process 837068 stopped * thread #1, name = 'a.out', stop reason = breakpoint 1.1 frame #0: 0x00005555555551bf a.out`main at hello.cc:4:13 1 #include <iostream> 2 3 int main() { -> 4 std::cout << "Hello World" << std::endl; 5 return 0; 6 } ```
1 parent cd83444 commit 7fb620a

File tree

2 files changed

+102
-14
lines changed

2 files changed

+102
-14
lines changed

lldb/source/Core/ModuleList.cpp

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -214,34 +214,38 @@ const ModuleList &ModuleList::operator=(const ModuleList &rhs) {
214214
ModuleList::~ModuleList() = default;
215215

216216
void ModuleList::AppendImpl(const ModuleSP &module_sp, bool use_notifier) {
217-
if (module_sp) {
217+
if (!module_sp)
218+
return;
219+
{
218220
std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
219221
// We are required to keep the first element of the Module List as the
220-
// executable module. So check here and if the first module is NOT an
221-
// but the new one is, we insert this module at the beginning, rather than
222+
// executable module. So check here and if the first module is NOT an
223+
// but the new one is, we insert this module at the beginning, rather than
222224
// at the end.
223225
// We don't need to do any of this if the list is empty:
224226
if (m_modules.empty()) {
225227
m_modules.push_back(module_sp);
226228
} else {
227-
// Since producing the ObjectFile may take some work, first check the 0th
228-
// element, and only if that's NOT an executable look at the incoming
229-
// ObjectFile. That way in the normal case we only look at the element
230-
// 0 ObjectFile.
231-
const bool elem_zero_is_executable
232-
= m_modules[0]->GetObjectFile()->GetType()
233-
== ObjectFile::Type::eTypeExecutable;
229+
// Since producing the ObjectFile may take some work, first check the
230+
// 0th element, and only if that's NOT an executable look at the
231+
// incoming ObjectFile. That way in the normal case we only look at the
232+
// element 0 ObjectFile.
233+
const bool elem_zero_is_executable =
234+
m_modules[0]->GetObjectFile()->GetType() ==
235+
ObjectFile::Type::eTypeExecutable;
234236
lldb_private::ObjectFile *obj = module_sp->GetObjectFile();
235-
if (!elem_zero_is_executable && obj
236-
&& obj->GetType() == ObjectFile::Type::eTypeExecutable) {
237+
if (!elem_zero_is_executable && obj &&
238+
obj->GetType() == ObjectFile::Type::eTypeExecutable) {
237239
m_modules.insert(m_modules.begin(), module_sp);
238240
} else {
239241
m_modules.push_back(module_sp);
240242
}
241243
}
242-
if (use_notifier && m_notifier)
243-
m_notifier->NotifyModuleAdded(*this, module_sp);
244244
}
245+
// Release the mutex before calling the notifier to avoid deadlock
246+
// NotifyModuleAdded should be thread-safe
247+
if (use_notifier && m_notifier)
248+
m_notifier->NotifyModuleAdded(*this, module_sp);
245249
}
246250

247251
void ModuleList::Append(const ModuleSP &module_sp, bool notify) {

lldb/test/API/functionalities/statusline/TestStatusline.py

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
import lldb
2+
import os
23
import re
4+
import socket
5+
import time
36

7+
from contextlib import closing
48
from lldbsuite.test.decorators import *
59
from lldbsuite.test.lldbtest import *
610
from lldbsuite.test.lldbpexpect import PExpectTest
11+
from lldbgdbserverutils import get_lldb_server_exe
712

813

914
# PExpect uses many timeouts internally and doesn't play well
@@ -115,3 +120,82 @@ def test_resize(self):
115120
# Check for the escape code to resize the scroll window.
116121
self.child.expect(re.escape("\x1b[1;19r"))
117122
self.child.expect("(lldb)")
123+
124+
@skipIfRemote
125+
@skipIfWindows
126+
@skipIfDarwin
127+
@add_test_categories(["lldb-server"])
128+
def test_modulelist_deadlock(self):
129+
"""Regression test for a deadlock that occurs when the status line is enabled before connecting
130+
to a gdb-remote server.
131+
"""
132+
if get_lldb_server_exe() is None:
133+
self.skipTest("lldb-server not found")
134+
135+
MAX_RETRY_ATTEMPTS = 10
136+
DELAY = 0.25
137+
138+
def _find_free_port(host):
139+
for attempt in range(MAX_RETRY_ATTEMPTS):
140+
try:
141+
family, type, proto, _, _ = socket.getaddrinfo(
142+
host, 0, proto=socket.IPPROTO_TCP
143+
)[0]
144+
with closing(socket.socket(family, type, proto)) as sock:
145+
sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
146+
sock.bind((host, 0))
147+
return sock.getsockname()
148+
except OSError:
149+
time.sleep(DELAY * 2**attempt) # Exponential backoff
150+
raise RuntimeError(
151+
"Could not find a free port on {} after {} attempts.".format(
152+
host, MAX_RETRY_ATTEMPTS
153+
)
154+
)
155+
156+
def _wait_for_server_ready_in_log(log_file_path, ready_message):
157+
"""Check log file for server ready message with retry logic"""
158+
for attempt in range(MAX_RETRY_ATTEMPTS):
159+
if os.path.exists(log_file_path):
160+
with open(log_file_path, "r") as f:
161+
if ready_message in f.read():
162+
return
163+
time.sleep(pow(2, attempt) * DELAY)
164+
raise RuntimeError(
165+
"Server not ready after {} attempts.".format(MAX_RETRY_ATTEMPTS)
166+
)
167+
168+
self.build()
169+
exe_path = self.getBuildArtifact("a.out")
170+
server_log_file = self.getLogBasenameForCurrentTest() + "-lldbserver.log"
171+
self.addTearDownHook(
172+
lambda: os.path.exists(server_log_file) and os.remove(server_log_file)
173+
)
174+
175+
# Find a free port for the server
176+
addr = _find_free_port("localhost")
177+
connect_address = "[{}]:{}".format(*addr)
178+
commandline_args = [
179+
"gdbserver",
180+
connect_address,
181+
exe_path,
182+
"--log-file={}".format(server_log_file),
183+
"--log-channels=lldb conn",
184+
]
185+
186+
server_proc = self.spawnSubprocess(
187+
get_lldb_server_exe(), commandline_args, install_remote=False
188+
)
189+
self.assertIsNotNone(server_proc)
190+
191+
# Wait for server to be ready by checking log file.
192+
server_ready_message = "Listen to {}".format(connect_address)
193+
_wait_for_server_ready_in_log(server_log_file, server_ready_message)
194+
195+
# Launch LLDB client and connect to lldb-server with statusline enabled
196+
self.launch(timeout=self.TIMEOUT)
197+
self.resize()
198+
self.expect("settings set show-statusline true", ["no target"])
199+
self.expect(
200+
f"gdb-remote {connect_address}", [b"a.out \xe2\x94\x82 signal SIGSTOP"]
201+
)

0 commit comments

Comments
 (0)