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.

Sign up for GitHub

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

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

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ab08de3
Fix a deadlock in ModuleList when starting a standalone lldb client
qxy11 Jul 15, 2025
a50d76a
Add a unit test
qxy11 Jul 17, 2025
24d8a4a
Refactor w/ early return
qxy11 Jul 17, 2025
a924424
Fix lints on the test
qxy11 Jul 17, 2025
bca8f55
Fix formatting
qxy11 Jul 17, 2025
5cf94e9
Fix unit test expect str
qxy11 Jul 18, 2025
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
46 changes: 25 additions & 21 deletions lldb/source/Core/ModuleList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,30 +215,34 @@ ModuleList::~ModuleList() = default;

void ModuleList::AppendImpl(const ModuleSP &module_sp, bool use_notifier) {
if (module_sp) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you're already touching this, let's make this an early return to offset the indentation for the lexical block.

Suggested change
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
// 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;
lldb_private::ObjectFile *obj = module_sp->GetObjectFile();
if (!elem_zero_is_executable && obj
&& obj->GetType() == ObjectFile::Type::eTypeExecutable) {
m_modules.insert(m_modules.begin(), module_sp);
} else {
{
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
// 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;
lldb_private::ObjectFile *obj = module_sp->GetObjectFile();
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);
}
}
}
// 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);
}
Expand Down
Loading