[Lldb-commits] [lldb] [lldb] Expose the Target API mutex through the SB API (PR #133295)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Mar 27 11:30:37 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
<details>
<summary>Changes</summary>
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
---
Full diff: https://github.com/llvm/llvm-project/pull/133295.diff
13 Files Affected:
- (added) lldb/bindings/interface/SBMutexExtensions.i (+12)
- (modified) lldb/bindings/interfaces.swig (+3-1)
- (modified) lldb/include/lldb/API/LLDB.h (+1)
- (modified) lldb/include/lldb/API/SBDefines.h (+1)
- (added) lldb/include/lldb/API/SBMutex.h (+48)
- (modified) lldb/include/lldb/API/SBTarget.h (+3-1)
- (modified) lldb/include/lldb/Target/Target.h (+16)
- (modified) lldb/source/API/CMakeLists.txt (+1)
- (added) lldb/source/API/SBMutex.cpp (+59)
- (modified) lldb/source/API/SBTarget.cpp (+12-4)
- (modified) lldb/test/API/python_api/target/TestTargetAPI.py (+24)
- (modified) lldb/unittests/API/CMakeLists.txt (+1)
- (added) lldb/unittests/API/SBMutexTest.cpp (+60)
``````````diff
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();
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/133295
More information about the lldb-commits
mailing list