Skip to content

Fix a deadlock in ModuleList when starting a standalone lldb client/server #148774

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

Merged
merged 6 commits into from
Aug 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 18 additions & 14 deletions lldb/source/Core/ModuleList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,34 +214,38 @@ const ModuleList &ModuleList::operator=(const ModuleList &rhs) {
ModuleList::~ModuleList() = default;

void ModuleList::AppendImpl(const ModuleSP &module_sp, bool use_notifier) {
if (module_sp) {
if (!module_sp)
return;
{
std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
// We are required to keep the first element of the Module List as the
// executable module. So check here and if the first module is NOT an
// but the new one is, we insert this module at the beginning, rather than
// executable module. So check here and if the first module is NOT an
// but the new one is, we insert this module at the beginning, rather than
// at the end.
// We don't need to do any of this if the list is empty:
if (m_modules.empty()) {
m_modules.push_back(module_sp);
} else {
// Since producing the ObjectFile may take some work, first check the 0th
// element, and only if that's NOT an executable look at the incoming
// ObjectFile. That way in the normal case we only look at the element
// 0 ObjectFile.
const bool elem_zero_is_executable
= m_modules[0]->GetObjectFile()->GetType()
== ObjectFile::Type::eTypeExecutable;
// Since producing the ObjectFile may take some work, first check the
// 0th element, and only if that's NOT an executable look at the
// incoming ObjectFile. That way in the normal case we only look at the
// element 0 ObjectFile.
const bool elem_zero_is_executable =
m_modules[0]->GetObjectFile()->GetType() ==
ObjectFile::Type::eTypeExecutable;
lldb_private::ObjectFile *obj = module_sp->GetObjectFile();
if (!elem_zero_is_executable && obj
&& obj->GetType() == ObjectFile::Type::eTypeExecutable) {
if (!elem_zero_is_executable && obj &&
obj->GetType() == ObjectFile::Type::eTypeExecutable) {
m_modules.insert(m_modules.begin(), module_sp);
} else {
m_modules.push_back(module_sp);
}
}
if (use_notifier && m_notifier)
m_notifier->NotifyModuleAdded(*this, module_sp);
}
// Release the mutex before calling the notifier to avoid deadlock
// NotifyModuleAdded should be thread-safe
if (use_notifier && m_notifier)
m_notifier->NotifyModuleAdded(*this, module_sp);
}

void ModuleList::Append(const ModuleSP &module_sp, bool notify) {
Expand Down
84 changes: 84 additions & 0 deletions lldb/test/API/functionalities/statusline/TestStatusline.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import lldb
import os
import re
import socket
import time

from contextlib import closing
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
from lldbsuite.test.lldbpexpect import PExpectTest
from lldbgdbserverutils import get_lldb_server_exe


# PExpect uses many timeouts internally and doesn't play well
Expand Down Expand Up @@ -115,3 +120,82 @@ def test_resize(self):
# Check for the escape code to resize the scroll window.
self.child.expect(re.escape("\x1b[1;19r"))
self.child.expect("(lldb)")

@skipIfRemote
@skipIfWindows
@skipIfDarwin
@add_test_categories(["lldb-server"])
def test_modulelist_deadlock(self):
"""Regression test for a deadlock that occurs when the status line is enabled before connecting
to a gdb-remote server.
"""
if get_lldb_server_exe() is None:
self.skipTest("lldb-server not found")

MAX_RETRY_ATTEMPTS = 10
DELAY = 0.25

def _find_free_port(host):
for attempt in range(MAX_RETRY_ATTEMPTS):
try:
family, type, proto, _, _ = socket.getaddrinfo(
host, 0, proto=socket.IPPROTO_TCP
)[0]
with closing(socket.socket(family, type, proto)) as sock:
sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
sock.bind((host, 0))
return sock.getsockname()
except OSError:
time.sleep(DELAY * 2**attempt) # Exponential backoff
raise RuntimeError(
"Could not find a free port on {} after {} attempts.".format(
host, MAX_RETRY_ATTEMPTS
)
)

def _wait_for_server_ready_in_log(log_file_path, ready_message):
"""Check log file for server ready message with retry logic"""
for attempt in range(MAX_RETRY_ATTEMPTS):
if os.path.exists(log_file_path):
with open(log_file_path, "r") as f:
if ready_message in f.read():
return
time.sleep(pow(2, attempt) * DELAY)
raise RuntimeError(
"Server not ready after {} attempts.".format(MAX_RETRY_ATTEMPTS)
)

self.build()
exe_path = self.getBuildArtifact("a.out")
server_log_file = self.getLogBasenameForCurrentTest() + "-lldbserver.log"
self.addTearDownHook(
lambda: os.path.exists(server_log_file) and os.remove(server_log_file)
)

# Find a free port for the server
addr = _find_free_port("localhost")
connect_address = "[{}]:{}".format(*addr)
commandline_args = [
"gdbserver",
connect_address,
exe_path,
"--log-file={}".format(server_log_file),
"--log-channels=lldb conn",
]

server_proc = self.spawnSubprocess(
get_lldb_server_exe(), commandline_args, install_remote=False
)
self.assertIsNotNone(server_proc)

# Wait for server to be ready by checking log file.
server_ready_message = "Listen to {}".format(connect_address)
_wait_for_server_ready_in_log(server_log_file, server_ready_message)

# Launch LLDB client and connect to lldb-server with statusline enabled
self.launch(timeout=self.TIMEOUT)
self.resize()
self.expect("settings set show-statusline true", ["no target"])
self.expect(
f"gdb-remote {connect_address}", [b"a.out \xe2\x94\x82 signal SIGSTOP"]
)
Loading