[Lldb-commits] [lldb] [lldb][dap] Avoid concurrent `HandleCommand` calls (PR #83162)
https://github.com/rupprecht closed https://github.com/llvm/llvm-project/pull/83162 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][dap] Avoid concurrent `HandleCommand` calls (PR #83162)
https://github.com/ZequanWu approved this pull request. https://github.com/llvm/llvm-project/pull/83162 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][dap] Avoid concurrent `HandleCommand` calls (PR #83162)
https://github.com/walter-erquinigo approved this pull request. This is amazing! That finally explains some flakes I've seen https://github.com/llvm/llvm-project/pull/83162 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][dap] Avoid concurrent `HandleCommand` calls (PR #83162)
llvmbot wrote:
@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
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
+
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 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 ==
``
https://github.com/llvm/llvm-project/pull/83162
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][dap] Avoid concurrent `HandleCommand` calls (PR #83162)
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
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 *
-@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
+
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 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 ==
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
