[Lldb-commits] [lldb] [lldb][dap] Avoid concurrent `HandleCommand` calls (PR #83162)

[Jordan Rupprecht via lldb-commits]

https://github.com/rupprecht created https://github.com/llvm/llvm-project/pull/83162

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.

>From 10104e6fcb0a279da202c02e242d69d92655128a Mon Sep 17 00:00:00 2001
From: Jordan Rupprecht <rupprecht at google.com>
Date: Tue, 27 Feb 2024 09:50:57 -0800
Subject: [PATCH] [lldb][dap] Avoid concurrent `HandleCommand` calls

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.

Prior to this, TestDAP_launch.py is 4% flaky. After, it passes in 1000 runs.
---
 .../API/tools/lldb-dap/commands/TestDAP_commands.py  |  2 --
 lldb/tools/lldb-dap/LLDBUtils.cpp                    | 12 +++++++++++-
 2 files changed, 11 insertions(+), 3 deletions(-)

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 *
 
 
- at 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 ==