[lldb] Expose the Target API mutex through the SB API (llvm#133295)

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 llvm#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]: llvm#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

(cherry picked from commit 50949eb)

Author: JDevlieghere

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
+}

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,15 +122,16 @@
 %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"
 %include "lldb/API/SBMemoryRegionInfo.h"
 %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"

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"

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;

lldb/include/lldb/API/SBMutex.h

@@ -0,0 +1,45 @@
+//===-- 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_SBMUTEX_H
+#define LLDB_API_SBMUTEX_H
+
+#include "lldb/API/SBDefines.h"
+#include "lldb/lldb-forward.h"
+#include <mutex>
+
+namespace lldb {
+
+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;
+};
+
+} // namespace lldb
+
+#endif

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;
@@ -957,6 +957,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;

lldb/source/API/CMakeLists.txt

@@ -92,6 +92,7 @@ add_lldb_library(liblldb SHARED ${option_framework}
   SBMemoryRegionInfoList.cpp
   SBModule.cpp
   SBModuleSpec.cpp
+  SBMutex.cpp
   SBPlatform.cpp
   SBProgress.cpp
   SBProcess.cpp

lldb/source/API/SBMutex.cpp

@@ -0,0 +1,60 @@
+//===-- 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() : m_opaque_sp(std::make_shared<std::recursive_mutex>()) {
+  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();
+}

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 "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBDebugger.h"
@@ -21,6 +17,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"
@@ -59,11 +56,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"
@@ -2497,3 +2497,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();
+}

lldb/test/API/python_api/target/TestTargetAPI.py

@@ -532,3 +532,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_get_api_mutex(self):
+        """Make sure we can lock and unlock the API mutex 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_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:
+            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")

lldb/unittests/API/CMakeLists.txt

@@ -1,5 +1,6 @@
 add_lldb_unittest(APITests
   SBCommandInterpreterTest.cpp
+  SBMutexTest.cpp
 
   LINK_LIBS
     liblldb

lldb/unittests/API/SBMutexTest.cpp

@@ -0,0 +1,57 @@
+//===-- 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();
+}