Skip to content

Conversation

@da-viper
Copy link
Contributor

The modules event requires the APIMutex to create the module load address. this may happen at the same time the module request is handled.

The modules request also requires the APIMutex and the `modules_mutex, set the order to acquire the mutexes.

@da-viper da-viper requested review from ashgti and removed request for JDevlieghere October 16, 2025 16:43
@da-viper da-viper requested a review from JDevlieghere October 16, 2025 16:43
@llvmbot
Copy link
Member

llvmbot commented Oct 16, 2025

@llvm/pr-subscribers-lldb

Author: Ebuka Ezike (da-viper)

Changes

The modules event requires the APIMutex to create the module load address. this may happen at the same time the module request is handled.

The modules request also requires the APIMutex and the `modules_mutex, set the order to acquire the mutexes.


Full diff: https://github.com/llvm/llvm-project/pull/163821.diff

1 Files Affected:

  • (modified) lldb/tools/lldb-dap/DAP.cpp (+6-1)
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index f76656e98ca01..003b55fa31610 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -1437,7 +1437,12 @@ void DAP::EventThread() {
           const bool remove_module =
               event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded;
 
-          std::lock_guard<std::mutex> guard(modules_mutex);
+          // NOTE: Both mutexes must be acquired (API mutex first, then modules
+          // mutex) to prevent deadlock when handling `modules_request`, which
+          // also requires both locks.
+          lldb::SBMutex api_mutex = GetAPIMutex();
+          std::lock_guard api_guard(api_mutex);
+          std::lock_guard modules_guard(modules_mutex);
           for (uint32_t i = 0; i < num_modules; ++i) {
             lldb::SBModule module =
                 lldb::SBTarget::GetModuleAtIndexFromEvent(i, event);

@da-viper da-viper force-pushed the fix-deadlock-modules branch from b10c3b8 to b1ae0f7 Compare October 20, 2025 20:41
Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

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

Thank you!

Copy link
Contributor

@ashgti ashgti left a comment

Choose a reason for hiding this comment

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

LGTM!

The modules event requires the `APIMutex` to create the module
load address. this may happen at the same time the `module
request` is handled.

The modules request also requires the `APIMutex` and the `modules_mutex,
set the order to acquire the mutexes.
@da-viper da-viper force-pushed the fix-deadlock-modules branch from d6c4a77 to fd2f356 Compare October 22, 2025 12:51
@da-viper da-viper merged commit f67880a into llvm:main Oct 22, 2025
10 checks passed
@da-viper da-viper deleted the fix-deadlock-modules branch October 22, 2025 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants