[lldb] Expose the Target API lock through the SB API

Author: JDevlieghere

lldb/bindings/interface/SBLockExtensions.i

@@ -0,0 +1,11 @@
+%extend lldb::SBLock {
+#ifdef SWIGPYTHON
+    %pythoncode %{
+        def __enter__(self):
+            return self
+
+        def __exit__(self, exc_type, exc_value, traceback):
+            self.Unlock()
+    %}
+#endif
+}

lldb/bindings/interfaces.swig

@@ -47,6 +47,7 @@
 %include "./interface/SBLaunchInfoDocstrings.i"
 %include "./interface/SBLineEntryDocstrings.i"
 %include "./interface/SBListenerDocstrings.i"
+%include "./interface/SBLockExtensions.i"
 %include "./interface/SBMemoryRegionInfoDocstrings.i"
 %include "./interface/SBMemoryRegionInfoListDocstrings.i"
 %include "./interface/SBModuleDocstrings.i"
@@ -121,11 +122,12 @@
 %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/SBLock.h"
 %include "lldb/API/SBMemoryRegionInfo.h"
 %include "lldb/API/SBMemoryRegionInfoList.h"
 %include "lldb/API/SBModule.h"

lldb/include/lldb/API/LLDB.h

@@ -46,6 +46,7 @@
 #include "lldb/API/SBLaunchInfo.h"
 #include "lldb/API/SBLineEntry.h"
 #include "lldb/API/SBListener.h"
+#include "lldb/API/SBLock.h"
 #include "lldb/API/SBMemoryRegionInfo.h"
 #include "lldb/API/SBMemoryRegionInfoList.h"
 #include "lldb/API/SBModule.h"

lldb/include/lldb/API/SBDefines.h

@@ -84,6 +84,7 @@ class LLDB_API SBLanguageRuntime;
 class LLDB_API SBLaunchInfo;
 class LLDB_API SBLineEntry;
 class LLDB_API SBListener;
+class LLDB_API SBLock;
 class LLDB_API SBMemoryRegionInfo;
 class LLDB_API SBMemoryRegionInfoList;
 class LLDB_API SBModule;

lldb/include/lldb/API/SBLock.h

@@ -0,0 +1,46 @@
+//===-- SBLock.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_private {
+class APILock;
+}
+
+namespace lldb {
+
+class LLDB_API SBLock {
+public:
+  SBLock();
+  SBLock(SBLock &&rhs);
+  SBLock &operator=(SBLock &&rhs);
+  ~SBLock();
+
+  bool IsValid() const;
+
+  void Unlock() const;
+
+private:
+  // Private constructor used by SBTarget to create the Target API lock.
+  // Requires a friend declaration.
+  SBLock(lldb::TargetSP target_sp);
+  friend class SBTarget;
+
+  SBLock(const SBLock &rhs) = delete;
+  const SBLock &operator=(const SBLock &rhs) = delete;
+
+  std::unique_ptr<lldb_private::APILock> m_opaque_up;
+};
+#endif
+
+} // namespace lldb

lldb/include/lldb/API/SBTarget.h

@@ -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::SBLock AcquireAPILock() const;
+
 protected:
   friend class SBAddress;
   friend class SBAddressRange;

lldb/include/lldb/Target/Target.h

@@ -1692,6 +1692,21 @@ 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 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

lldb/source/API/CMakeLists.txt

@@ -77,6 +77,7 @@ add_lldb_library(liblldb SHARED ${option_framework}
   SBLaunchInfo.cpp
   SBLineEntry.cpp
   SBListener.cpp
+  SBLock.cpp
   SBMemoryRegionInfo.cpp
   SBMemoryRegionInfoList.cpp
   SBModule.cpp

lldb/source/API/SBLock.cpp

@@ -0,0 +1,52 @@
+//===-- SBLock.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/SBLock.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;
+
+SBLock::SBLock() { LLDB_INSTRUMENT_VA(this); }
+
+SBLock::SBLock(SBLock &&rhs) : m_opaque_up(std::move(rhs.m_opaque_up)) {
+  LLDB_INSTRUMENT_VA(this);
+}
+
+SBLock &SBLock::operator=(SBLock &&rhs) {
+  LLDB_INSTRUMENT_VA(this);
+
+  m_opaque_up = std::move(rhs.m_opaque_up);
+  return *this;
+}
+
+SBLock::SBLock(lldb::TargetSP target_sp)
+    : m_opaque_up(
+          std::make_unique<APILock>(std::shared_ptr<std::recursive_mutex>(
+              target_sp, &target_sp->GetAPIMutex()))) {
+  LLDB_INSTRUMENT_VA(this, target_sp);
+}
+
+SBLock::~SBLock() { LLDB_INSTRUMENT_VA(this); }
+
+bool SBLock::IsValid() const {
+  LLDB_INSTRUMENT_VA(this);
+
+  return static_cast<bool>(m_opaque_up) && static_cast<bool>(*m_opaque_up);
+}
+
+void SBLock::Unlock() const {
+  LLDB_INSTRUMENT_VA(this);
+
+  if (m_opaque_up)
+    m_opaque_up->Unlock();
+}

lldb/source/API/SBTarget.cpp

@@ -7,17 +7,14 @@
 //===----------------------------------------------------------------------===//
 
 #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"
 #include "lldb/API/SBEvent.h"
 #include "lldb/API/SBExpressionOptions.h"
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBListener.h"
+#include "lldb/API/SBLock.h"
 #include "lldb/API/SBModule.h"
 #include "lldb/API/SBModuleSpec.h"
 #include "lldb/API/SBProcess.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::SBLock SBTarget::AcquireAPILock() const {
+  LLDB_INSTRUMENT_VA(this);
+
+  if (TargetSP target_sp = GetSP())
+    return lldb::SBLock(target_sp);
+  return lldb::SBLock();
+}

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

@@ -537,3 +537,31 @@ 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()
+
+        lock = target.AcquireAPILock()
+        self.assertTrue(lock.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")
+        lock.Unlock()
+        self.assertFalse(lock.IsValid())
+
+    @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()
+
+        lock_copy = None
+        with target.AcquireAPILock() as lock:
+            self.assertTrue(lock.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")
+            lock_copy = lock
+            self.assertTrue(lock.IsValid())
+        self.assertFalse(lock_copy.IsValid())

lldb/unittests/API/CMakeLists.txt

@@ -1,6 +1,7 @@
 add_lldb_unittest(APITests
   SBCommandInterpreterTest.cpp
   SBLineEntryTest.cpp
+  SBLockTest.cpp
 
   LINK_LIBS
     liblldb

lldb/unittests/API/SBLockTest.cpp

@@ -0,0 +1,57 @@
+//===-- SBLockTest.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>
+
+using namespace lldb;
+using namespace lldb_private;
+
+class SBLockTest : public testing::Test {
+protected:
+  void SetUp() override { debugger = SBDebugger::Create(); }
+  void TearDown() override { SBDebugger::Destroy(debugger); }
+
+  SubsystemRAII<lldb::SBDebugger> subsystems;
+  SBDebugger debugger;
+};
+
+TEST_F(SBLockTest, LockTest) {
+  lldb::SBTarget target = debugger.GetDummyTarget();
+
+  std::future<void> f;
+  {
+    std::atomic<bool> locked = false;
+    lldb::SBLock lock = target.AcquireAPILock();
+    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();
+}