[lldb][dap] Avoid concurrent HandleCommand calls

[rupprecht]

The EventThreadFunction can end up calling HandleCommand concurrently with the main request processing thread. The underlying API does not appear to be thread safe, so add a narrowly scoped mutex lock to prevent calling it in this place from more than one thread.

Fixes #81686. Prior to this, TestDAP_launch.py is 4% flaky. After, it passes in 1000 runs.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jordan Rupprecht (rupprecht)

Changes

The EventThreadFunction can end up calling HandleCommand concurrently with the main request processing thread. The underlying API does not appear to be thread safe, so add a narrowly scoped mutex lock to prevent calling it in this place from more than one thread.

Fixes #81686. Prior to this, TestDAP_launch.py is 4% flaky. After, it passes in 1000 runs.

---

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

2 Files Affected:

• (modified) lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py (-2)
• (modified) lldb/tools/lldb-dap/LLDBUtils.cpp (+11-1)

diff --git a/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py b/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py
index 8dcda75d0a4520..226b9385fe719a 100644
--- a/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py
+++ b/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py
@@ -1,5 +1,4 @@
 import os
-import unittest
 
 import dap_server
 import lldbdap_testcase
@@ -7,7 +6,6 @@
 from lldbsuite.test.decorators import *
 
 
-@unittest.skip("https://llvm.org/PR81686")
 class TestDAP_commands(lldbdap_testcase.DAPTestCaseBase):
     def test_command_directive_quiet_on_success(self):
         program = self.getBuildArtifact("a.out")
diff --git a/lldb/tools/lldb-dap/LLDBUtils.cpp b/lldb/tools/lldb-dap/LLDBUtils.cpp
index 35b7a986a8964b..a91cc6718f4dfc 100644
--- a/lldb/tools/lldb-dap/LLDBUtils.cpp
+++ b/lldb/tools/lldb-dap/LLDBUtils.cpp
@@ -9,6 +9,8 @@
 #include "LLDBUtils.h"
 #include "DAP.h"
 
+#include <mutex>
+
 namespace lldb_dap {
 
 bool RunLLDBCommands(llvm::StringRef prefix,
@@ -37,7 +39,15 @@ bool RunLLDBCommands(llvm::StringRef prefix,
       }
     }
 
-    interp.HandleCommand(command.str().c_str(), result);
+    {
+      // Prevent simultaneous calls to HandleCommand, e.g. EventThreadFunction
+      // may asynchronously call RunExitCommands when we are already calling
+      // RunTerminateCommands.
+      static std::mutex handle_command_mutex;
+      std::lock_guard<std::mutex> locker(handle_command_mutex);
+      interp.HandleCommand(command.str().c_str(), result);
+    }
+
     const bool got_error = !result.Succeeded();
     // The if statement below is assuming we always print out `!` prefixed
     // lines. The only time we don't print is when we have `quiet_on_success ==

[walter-erquinigo]

This is amazing! That finally explains some flakes I've seen