[Lldb-commits] [lldb] [lldb] Expose the Target API mutex through the SB API (PR #133295)
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Thu Mar 27 11:32:48 PDT 2025
https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/133295
>From 79241d2f5701ab789f7fdbb5bd881c4494cb67eb Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Thu, 27 Mar 2025 11:24:50 -0700
Subject: [PATCH 1/2] [lldb] Expose the Target API mutex through the SB API
Expose u target API mutex through the SB API. This is motivated by
lldb-dap, which is built on top of the SB API and needs a way to execute
a series of SB API calls in an atomic manner (see #131242).
We can solve this problem by either introducing an additional layer of
locking at the DAP level or by exposing the existing locking at the SB
API level. This patch implements the second approach.
This was discussed in an RFC on Discourse [0]. The original
implementation exposed a move-only lock rather than a mutex [1] which
doesn't work well with SWIG 4.0 [2]. This implement the alternative
solution of exposing the mutex rather than the lock. The SBMutex
conforms to the BasicLockable requirement [3] (which is why the methods
are called `lock` and `unlock` rather than Lock and Unlock) so it can be
used as `std::lock_guard<lldb::SBMutex>` and
`std::unique_lock<lldb::SBMutex>`.
[0]: https://discourse.llvm.org/t/rfc-exposing-the-target-api-lock-through-the-sb-api/85215/6
[1]: https://github.com/llvm/llvm-project/pull/131404
[2]: https://discourse.llvm.org/t/rfc-bumping-the-minimum-swig-version-to-4-1-0/85377/9
[3]: https://en.cppreference.com/w/cpp/named_req/BasicLockable
---
lldb/bindings/interface/SBMutexExtensions.i | 12 ++++
lldb/bindings/interfaces.swig | 4 +-
lldb/include/lldb/API/LLDB.h | 1 +
lldb/include/lldb/API/SBDefines.h | 1 +
lldb/include/lldb/API/SBMutex.h | 48 +++++++++++++++
lldb/include/lldb/API/SBTarget.h | 4 +-
lldb/include/lldb/Target/Target.h | 16 +++++
lldb/source/API/CMakeLists.txt | 1 +
lldb/source/API/SBMutex.cpp | 59 ++++++++++++++++++
lldb/source/API/SBTarget.cpp | 16 +++--
.../API/python_api/target/TestTargetAPI.py | 24 ++++++++
lldb/unittests/API/CMakeLists.txt | 1 +
lldb/unittests/API/SBMutexTest.cpp | 60 +++++++++++++++++++
13 files changed, 241 insertions(+), 6 deletions(-)
create mode 100644 lldb/bindings/interface/SBMutexExtensions.i
create mode 100644 lldb/include/lldb/API/SBMutex.h
create mode 100644 lldb/source/API/SBMutex.cpp
create mode 100644 lldb/unittests/API/SBMutexTest.cpp
diff --git a/lldb/bindings/interface/SBMutexExtensions.i b/lldb/bindings/interface/SBMutexExtensions.i
new file mode 100644
index 0000000000000..32d3fee468697
--- /dev/null
+++ b/lldb/bindings/interface/SBMutexExtensions.i
@@ -0,0 +1,12 @@
+%extend lldb::SBMutex {
+#ifdef SWIGPYTHON
+ %pythoncode %{
+ def __enter__(self):
+ self.lock()
+ return self
+
+ def __exit__(self, exc_type, exc_value, traceback):
+ self.unlock()
+ %}
+#endif
+}
diff --git a/lldb/bindings/interfaces.swig b/lldb/bindings/interfaces.swig
index 08df9a1a8d539..e36f2f9dd27c6 100644
--- a/lldb/bindings/interfaces.swig
+++ b/lldb/bindings/interfaces.swig
@@ -51,6 +51,7 @@
%include "./interface/SBMemoryRegionInfoListDocstrings.i"
%include "./interface/SBModuleDocstrings.i"
%include "./interface/SBModuleSpecDocstrings.i"
+%include "./interface/SBMutexExtensions.i"
%include "./interface/SBPlatformDocstrings.i"
%include "./interface/SBProcessDocstrings.i"
%include "./interface/SBProcessInfoDocstrings.i"
@@ -121,8 +122,8 @@
%include "lldb/API/SBHostOS.h"
%include "lldb/API/SBInstruction.h"
%include "lldb/API/SBInstructionList.h"
-%include "lldb/API/SBLanguages.h"
%include "lldb/API/SBLanguageRuntime.h"
+%include "lldb/API/SBLanguages.h"
%include "lldb/API/SBLaunchInfo.h"
%include "lldb/API/SBLineEntry.h"
%include "lldb/API/SBListener.h"
@@ -130,6 +131,7 @@
%include "lldb/API/SBMemoryRegionInfoList.h"
%include "lldb/API/SBModule.h"
%include "lldb/API/SBModuleSpec.h"
+%include "lldb/API/SBMutex.h"
%include "lldb/API/SBPlatform.h"
%include "lldb/API/SBProcess.h"
%include "lldb/API/SBProcessInfo.h"
diff --git a/lldb/include/lldb/API/LLDB.h b/lldb/include/lldb/API/LLDB.h
index 126fcef31b416..6485f35302a1c 100644
--- a/lldb/include/lldb/API/LLDB.h
+++ b/lldb/include/lldb/API/LLDB.h
@@ -50,6 +50,7 @@
#include "lldb/API/SBMemoryRegionInfoList.h"
#include "lldb/API/SBModule.h"
#include "lldb/API/SBModuleSpec.h"
+#include "lldb/API/SBMutex.h"
#include "lldb/API/SBPlatform.h"
#include "lldb/API/SBProcess.h"
#include "lldb/API/SBProcessInfo.h"
diff --git a/lldb/include/lldb/API/SBDefines.h b/lldb/include/lldb/API/SBDefines.h
index ed5a80da117a5..85f6bbeea5bf9 100644
--- a/lldb/include/lldb/API/SBDefines.h
+++ b/lldb/include/lldb/API/SBDefines.h
@@ -89,6 +89,7 @@ class LLDB_API SBMemoryRegionInfoList;
class LLDB_API SBModule;
class LLDB_API SBModuleSpec;
class LLDB_API SBModuleSpecList;
+class LLDB_API SBMutex;
class LLDB_API SBPlatform;
class LLDB_API SBPlatformConnectOptions;
class LLDB_API SBPlatformShellCommand;
diff --git a/lldb/include/lldb/API/SBMutex.h b/lldb/include/lldb/API/SBMutex.h
new file mode 100644
index 0000000000000..b2cd33a40a6de
--- /dev/null
+++ b/lldb/include/lldb/API/SBMutex.h
@@ -0,0 +1,48 @@
+//===-- SBMutex.h
+//----------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_API_SBLOCK_H
+#define LLDB_API_SBLOCK_H
+
+#include "lldb/API/SBDefines.h"
+#include "lldb/lldb-forward.h"
+#include <mutex>
+
+namespace lldb {
+
+/// A general-purpose lock in the SB API. The lock can be locked and unlocked.
+/// The default constructed lock is unlocked, but generally the lock is locked
+/// when it is returned from a class.
+class LLDB_API SBMutex {
+public:
+ SBMutex();
+ SBMutex(const SBMutex &rhs);
+ const SBMutex &operator=(const SBMutex &rhs);
+ ~SBMutex();
+
+ /// Returns true if this lock has ownership of the underlying mutex.
+ bool IsValid() const;
+
+ /// Blocking operation that takes ownership of this lock.
+ void lock() const;
+
+ /// Releases ownership of this lock.
+ void unlock() const;
+
+private:
+ // Private constructor used by SBTarget to create the Target API mutex.
+ // Requires a friend declaration.
+ SBMutex(lldb::TargetSP target_sp);
+ friend class SBTarget;
+
+ std::shared_ptr<std::recursive_mutex> m_opaque_sp;
+};
+#endif
+
+} // namespace lldb
diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h
index bb912ab41d0fe..17735fdca6559 100644
--- a/lldb/include/lldb/API/SBTarget.h
+++ b/lldb/include/lldb/API/SBTarget.h
@@ -342,7 +342,7 @@ class LLDB_API SBTarget {
uint32_t GetAddressByteSize();
const char *GetTriple();
-
+
const char *GetABIName();
const char *GetLabel() const;
@@ -946,6 +946,8 @@ class LLDB_API SBTarget {
/// An error if a Trace already exists or the trace couldn't be created.
lldb::SBTrace CreateTrace(SBError &error);
+ lldb::SBMutex GetAPIMutex() const;
+
protected:
friend class SBAddress;
friend class SBAddressRange;
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 3cdbe9221a0bc..d93020b60d711 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1692,6 +1692,22 @@ class Target : public std::enable_shared_from_this<Target>,
}
};
+/// The private implementation backing SBLock.
+class APILock {
+public:
+ APILock(std::shared_ptr<std::recursive_mutex> mutex_sp)
+ : m_mutex(std::move(mutex_sp)), m_lock(*m_mutex) {}
+
+ void Lock() { m_lock.lock(); }
+ void Unlock() { m_lock.unlock(); }
+
+ operator bool() const { return static_cast<bool>(m_lock); }
+
+private:
+ std::shared_ptr<std::recursive_mutex> m_mutex;
+ std::unique_lock<std::recursive_mutex> m_lock;
+};
+
} // namespace lldb_private
#endif // LLDB_TARGET_TARGET_H
diff --git a/lldb/source/API/CMakeLists.txt b/lldb/source/API/CMakeLists.txt
index 48d5cde5bf592..3bc569608e458 100644
--- a/lldb/source/API/CMakeLists.txt
+++ b/lldb/source/API/CMakeLists.txt
@@ -81,6 +81,7 @@ add_lldb_library(liblldb SHARED ${option_framework}
SBMemoryRegionInfoList.cpp
SBModule.cpp
SBModuleSpec.cpp
+ SBMutex.cpp
SBPlatform.cpp
SBProcess.cpp
SBProcessInfo.cpp
diff --git a/lldb/source/API/SBMutex.cpp b/lldb/source/API/SBMutex.cpp
new file mode 100644
index 0000000000000..59883feeb7497
--- /dev/null
+++ b/lldb/source/API/SBMutex.cpp
@@ -0,0 +1,59 @@
+//===-- SBMutex.cpp
+//--------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/API/SBMutex.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/Instrumentation.h"
+#include "lldb/lldb-forward.h"
+#include <memory>
+#include <mutex>
+
+using namespace lldb;
+using namespace lldb_private;
+
+SBMutex::SBMutex() { LLDB_INSTRUMENT_VA(this); }
+
+SBMutex::SBMutex(const SBMutex &rhs) : m_opaque_sp(rhs.m_opaque_sp) {
+ LLDB_INSTRUMENT_VA(this);
+}
+
+const SBMutex &SBMutex::operator=(const SBMutex &rhs) {
+ LLDB_INSTRUMENT_VA(this);
+
+ m_opaque_sp = rhs.m_opaque_sp;
+ return *this;
+}
+
+SBMutex::SBMutex(lldb::TargetSP target_sp)
+ : m_opaque_sp(std::shared_ptr<std::recursive_mutex>(
+ target_sp, &target_sp->GetAPIMutex())) {
+ LLDB_INSTRUMENT_VA(this, target_sp);
+}
+
+SBMutex::~SBMutex() { LLDB_INSTRUMENT_VA(this); }
+
+bool SBMutex::IsValid() const {
+ LLDB_INSTRUMENT_VA(this);
+
+ return static_cast<bool>(m_opaque_sp);
+}
+
+void SBMutex::lock() const {
+ LLDB_INSTRUMENT_VA(this);
+
+ if (m_opaque_sp)
+ m_opaque_sp->lock();
+}
+
+void SBMutex::unlock() const {
+ LLDB_INSTRUMENT_VA(this);
+
+ if (m_opaque_sp)
+ m_opaque_sp->unlock();
+}
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index dd9caa724ea36..0fed1bbfed6a7 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -7,10 +7,6 @@
//===----------------------------------------------------------------------===//
#include "lldb/API/SBTarget.h"
-#include "lldb/Utility/Instrumentation.h"
-#include "lldb/Utility/LLDBLog.h"
-#include "lldb/lldb-public.h"
-
#include "lldb/API/SBBreakpoint.h"
#include "lldb/API/SBDebugger.h"
#include "lldb/API/SBEnvironment.h"
@@ -20,6 +16,7 @@
#include "lldb/API/SBListener.h"
#include "lldb/API/SBModule.h"
#include "lldb/API/SBModuleSpec.h"
+#include "lldb/API/SBMutex.h"
#include "lldb/API/SBProcess.h"
#include "lldb/API/SBSourceManager.h"
#include "lldb/API/SBStream.h"
@@ -58,11 +55,14 @@
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/Args.h"
#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Instrumentation.h"
+#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/ProcessInfo.h"
#include "lldb/Utility/RegularExpression.h"
#include "lldb/ValueObject/ValueObjectConstResult.h"
#include "lldb/ValueObject/ValueObjectList.h"
#include "lldb/ValueObject/ValueObjectVariable.h"
+#include "lldb/lldb-public.h"
#include "Commands/CommandObjectBreakpoint.h"
#include "lldb/Interpreter/CommandReturnObject.h"
@@ -2439,3 +2439,11 @@ lldb::SBTrace SBTarget::CreateTrace(lldb::SBError &error) {
}
return SBTrace();
}
+
+lldb::SBMutex SBTarget::GetAPIMutex() const {
+ LLDB_INSTRUMENT_VA(this);
+
+ if (TargetSP target_sp = GetSP())
+ return lldb::SBMutex(target_sp);
+ return lldb::SBMutex();
+}
diff --git a/lldb/test/API/python_api/target/TestTargetAPI.py b/lldb/test/API/python_api/target/TestTargetAPI.py
index 155a25b576b03..523ba299abe18 100644
--- a/lldb/test/API/python_api/target/TestTargetAPI.py
+++ b/lldb/test/API/python_api/target/TestTargetAPI.py
@@ -537,3 +537,27 @@ def test_setting_selected_target_with_invalid_target(self):
"""Make sure we don't crash when trying to select invalid target."""
target = lldb.SBTarget()
self.dbg.SetSelectedTarget(target)
+
+ @no_debug_info_test
+ def test_acquire_sblock(self):
+ """Make sure we can acquire the API lock from Python."""
+ target = self.dbg.GetDummyTarget()
+
+ mutex = target.GetAPIMutex()
+ self.assertTrue(mutex.IsValid())
+ mutex.lock()
+ # The API call below doesn't actually matter, it's just there to
+ # confirm we don't block on the API lock.
+ target.BreakpointCreateByName("foo", "bar")
+ mutex.unlock()
+
+ @no_debug_info_test
+ def test_acquire_sblock_with_statement(self):
+ """Make sure we can acquire the API lock using a with-statement from Python."""
+ target = self.dbg.GetDummyTarget()
+
+ with target.GetAPIMutex() as mutex:
+ self.assertTrue(mutex.IsValid())
+ # The API call below doesn't actually matter, it's just there to
+ # confirm we don't block on the API lock.
+ target.BreakpointCreateByName("foo", "bar")
diff --git a/lldb/unittests/API/CMakeLists.txt b/lldb/unittests/API/CMakeLists.txt
index fe2ff684a5d92..8bdc806878239 100644
--- a/lldb/unittests/API/CMakeLists.txt
+++ b/lldb/unittests/API/CMakeLists.txt
@@ -1,6 +1,7 @@
add_lldb_unittest(APITests
SBCommandInterpreterTest.cpp
SBLineEntryTest.cpp
+ SBMutexTest.cpp
LINK_LIBS
liblldb
diff --git a/lldb/unittests/API/SBMutexTest.cpp b/lldb/unittests/API/SBMutexTest.cpp
new file mode 100644
index 0000000000000..e8ec07c586dfd
--- /dev/null
+++ b/lldb/unittests/API/SBMutexTest.cpp
@@ -0,0 +1,60 @@
+//===-- SBMutexTest.cpp
+//----------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===/
+
+// Use the umbrella header for -Wdocumentation.
+#include "lldb/API/LLDB.h"
+
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBTarget.h"
+#include "gtest/gtest.h"
+#include <atomic>
+#include <chrono>
+#include <future>
+#include <mutex>
+
+using namespace lldb;
+using namespace lldb_private;
+
+class SBMutexTest : public testing::Test {
+protected:
+ void SetUp() override { debugger = SBDebugger::Create(); }
+ void TearDown() override { SBDebugger::Destroy(debugger); }
+
+ SubsystemRAII<lldb::SBDebugger> subsystems;
+ SBDebugger debugger;
+};
+
+TEST_F(SBMutexTest, LockTest) {
+ lldb::SBTarget target = debugger.GetDummyTarget();
+
+ std::future<void> f;
+ {
+ std::atomic<bool> locked = false;
+ lldb::SBMutex lock = target.GetAPIMutex();
+ std::lock_guard<lldb::SBMutex> lock_guard(lock);
+ ASSERT_FALSE(locked.exchange(true));
+
+ f = std::async(std::launch::async, [&]() {
+ {
+ ASSERT_TRUE(locked);
+ target.BreakpointCreateByName("foo", "bar");
+ ASSERT_FALSE(locked);
+ }
+ });
+ ASSERT_TRUE(f.valid());
+
+ // Wait 500ms to confirm the thread is blocked.
+ auto status = f.wait_for(std::chrono::milliseconds(500));
+ ASSERT_EQ(status, std::future_status::timeout);
+
+ ASSERT_TRUE(locked.exchange(false));
+ }
+ f.wait();
+}
>From 088d513b836708e75ef98f2fa8f1f5c123fe3cc9 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Thu, 27 Mar 2025 11:31:43 -0700
Subject: [PATCH 2/2] Update test method names
---
lldb/test/API/python_api/target/TestTargetAPI.py | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lldb/test/API/python_api/target/TestTargetAPI.py b/lldb/test/API/python_api/target/TestTargetAPI.py
index 523ba299abe18..67b9d192bc625 100644
--- a/lldb/test/API/python_api/target/TestTargetAPI.py
+++ b/lldb/test/API/python_api/target/TestTargetAPI.py
@@ -539,8 +539,8 @@ def test_setting_selected_target_with_invalid_target(self):
self.dbg.SetSelectedTarget(target)
@no_debug_info_test
- def test_acquire_sblock(self):
- """Make sure we can acquire the API lock from Python."""
+ def test_get_api_mutex(self):
+ """Make sure we can lock and unlock the API mutex from Python."""
target = self.dbg.GetDummyTarget()
mutex = target.GetAPIMutex()
@@ -552,8 +552,8 @@ def test_acquire_sblock(self):
mutex.unlock()
@no_debug_info_test
- def test_acquire_sblock_with_statement(self):
- """Make sure we can acquire the API lock using a with-statement from Python."""
+ def test_get_api_mutex_with_statement(self):
+ """Make sure we can lock and unlock the API mutex using a with-statement from Python."""
target = self.dbg.GetDummyTarget()
with target.GetAPIMutex() as mutex:
More information about the lldb-commits
mailing list