SBDebugger: Add new APIs AddDestroyCallback and RemoveDestroyCallback

lldb/include/lldb/API/SBDebugger.h

@@ -194,7 +194,7 @@ class LLDB_API SBDebugger {
   lldb::SBCommandInterpreter GetCommandInterpreter();
 
   void HandleCommand(const char *command);
-  
+
   void RequestInterrupt();
   void CancelInterruptRequest();
   bool InterruptRequested();
@@ -321,8 +321,15 @@ class LLDB_API SBDebugger {
 
   void SetLoggingCallback(lldb::LogOutputCallback log_callback, void *baton);
 
-  void SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
-                          void *baton);
+  lldb::SBDebuggerDestroyCallbackToken
+  AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
+                     void *baton);
+
+  lldb::SBDebuggerDestroyCallbackToken
+  SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
+                     void *baton);
+
+  bool RemoveDestroyCallback(lldb::SBDebuggerDestroyCallbackToken);
 
 #ifndef SWIG
   LLDB_DEPRECATED_FIXME("Use DispatchInput(const void *, size_t)",

lldb/include/lldb/API/SBDefines.h

@@ -139,6 +139,8 @@ typedef bool (*SBBreakpointHitCallback)(void *baton, SBProcess &process,
 typedef void (*SBDebuggerDestroyCallback)(lldb::user_id_t debugger_id,
                                           void *baton);
 
+typedef int SBDebuggerDestroyCallbackToken;
+
 typedef SBError (*SBPlatformLocateModuleCallback)(
     void *baton, const SBModuleSpec &module_spec, SBFileSpec &module_file_spec,
     SBFileSpec &symbol_file_spec);

lldb/include/lldb/Core/Debugger.h

@@ -13,6 +13,7 @@
 
 #include <memory>
 #include <optional>
+#include <unordered_map>
 #include <vector>
 
 #include "lldb/Core/DebuggerEvents.h"
@@ -568,10 +569,22 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
 
   static void ReportSymbolChange(const ModuleSpec &module_spec);
 
-  void
+  /// DEPRECATED. Use AddDestroyCallback and RemoveDestroyCallback instead.
+  /// Clear all previously added callbacks and only add the given one.
+  lldb_private::DebuggerDestroyCallbackToken
   SetDestroyCallback(lldb_private::DebuggerDestroyCallback destroy_callback,
                      void *baton);
 
+  /// Add a callback for when the debugger is destroyed. Return a token, which
+  /// can be used to remove said callback. Multiple callbacks can be added by
+  /// calling this function multiple times.
+  lldb_private::DebuggerDestroyCallbackToken
+  AddDestroyCallback(lldb_private::DebuggerDestroyCallback destroy_callback,
+                     void *baton);
+
+  /// Remove the specified callback. Return true if successful.
+  bool RemoveDestroyCallback(lldb_private::DebuggerDestroyCallbackToken token);
+
   /// Manually start the global event handler thread. It is useful to plugins
   /// that directly use the \a lldb_private namespace and want to use the
   /// debugger's default event handler thread instead of defining their own.
@@ -731,8 +744,11 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
   lldb::TargetSP m_dummy_target_sp;
   Diagnostics::CallbackID m_diagnostics_callback_id;
 
-  lldb_private::DebuggerDestroyCallback m_destroy_callback = nullptr;
-  void *m_destroy_callback_baton = nullptr;
+  std::recursive_mutex m_destroy_callback_mutex;
+  lldb_private::DebuggerDestroyCallbackToken m_destroy_callback_next_token = 0;
+  std::unordered_map<lldb_private::DebuggerDestroyCallbackToken,
+                     std::pair<lldb_private::DebuggerDestroyCallback, void *>>
+      m_destroy_callback_and_baton;
 
   uint32_t m_interrupt_requested = 0; ///< Tracks interrupt requests
   std::mutex m_interrupt_mutex;

lldb/include/lldb/lldb-private-types.h

@@ -121,6 +121,7 @@ typedef struct type256 { uint64_t x[4]; } type256;
 using ValueObjectProviderTy =
     std::function<lldb::ValueObjectSP(ConstString, StackFrame *)>;
 
+typedef int DebuggerDestroyCallbackToken;
 typedef void (*DebuggerDestroyCallback)(lldb::user_id_t debugger_id,
                                         void *baton);
 typedef bool (*CommandOverrideCallbackWithResult)(

lldb/source/API/SBDebugger.cpp

@@ -1686,13 +1686,33 @@ void SBDebugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
   }
 }
 
-void SBDebugger::SetDestroyCallback(
-    lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) {
+lldb::SBDebuggerDestroyCallbackToken
+SBDebugger::AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) {
+  LLDB_INSTRUMENT_VA(this, destroy_callback, baton);
+  if (m_opaque_sp) {
+    return m_opaque_sp->AddDestroyCallback(
+        destroy_callback, baton);
+  }
+  return -1;
+}
+
+lldb::SBDebuggerDestroyCallbackToken
+SBDebugger::SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) {
   LLDB_INSTRUMENT_VA(this, destroy_callback, baton);
   if (m_opaque_sp) {
     return m_opaque_sp->SetDestroyCallback(
         destroy_callback, baton);
   }
+  return -1;
+}
+
+bool SBDebugger::RemoveDestroyCallback(
+    lldb::SBDebuggerDestroyCallbackToken token) {
+  LLDB_INSTRUMENT_VA(this);
+  if (m_opaque_sp) {
+    return m_opaque_sp->RemoveDestroyCallback(token);
+  }
+  return false;
 }
 
 SBTrace
@@ -1704,20 +1724,20 @@ SBDebugger::LoadTraceFromFile(SBError &error,
 
 void SBDebugger::RequestInterrupt() {
   LLDB_INSTRUMENT_VA(this);
-  
+
   if (m_opaque_sp)
-    m_opaque_sp->RequestInterrupt();  
+    m_opaque_sp->RequestInterrupt();
 }
 void SBDebugger::CancelInterruptRequest()  {
   LLDB_INSTRUMENT_VA(this);
-  
+
   if (m_opaque_sp)
-    m_opaque_sp->CancelInterruptRequest();  
+    m_opaque_sp->CancelInterruptRequest();
 }
 
 bool SBDebugger::InterruptRequested()   {
   LLDB_INSTRUMENT_VA(this);
-  
+
   if (m_opaque_sp)
     return m_opaque_sp->InterruptRequested();
   return false;

lldb/source/Core/Debugger.cpp

@@ -743,10 +743,14 @@ DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback,
 }
 
 void Debugger::HandleDestroyCallback() {
-  if (m_destroy_callback) {
-    m_destroy_callback(GetID(), m_destroy_callback_baton);
-    m_destroy_callback = nullptr;
+  std::lock_guard<std::recursive_mutex> guard(m_destroy_callback_mutex);
+  const lldb::user_id_t user_id = GetID();
+  for (const auto &element : m_destroy_callback_and_baton) {
+    const auto &callback = element.second.first;
+    const auto &baton = element.second.second;
+    callback(user_id, baton);
   }
+  m_destroy_callback_and_baton.clear();
 }
 
 void Debugger::Destroy(DebuggerSP &debugger_sp) {
@@ -1425,10 +1429,27 @@ void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
       std::make_shared<CallbackLogHandler>(log_callback, baton);
 }
 
-void Debugger::SetDestroyCallback(
+lldb_private::DebuggerDestroyCallbackToken Debugger::SetDestroyCallback(
     lldb_private::DebuggerDestroyCallback destroy_callback, void *baton) {
-  m_destroy_callback = destroy_callback;
-  m_destroy_callback_baton = baton;
+  std::lock_guard<std::recursive_mutex> guard(m_destroy_callback_mutex);
+  m_destroy_callback_and_baton.clear();
+  return AddDestroyCallback(destroy_callback, baton);
+}
+
+lldb_private::DebuggerDestroyCallbackToken Debugger::AddDestroyCallback(
+    lldb_private::DebuggerDestroyCallback destroy_callback, void *baton) {
+  std::lock_guard<std::recursive_mutex> guard(m_destroy_callback_mutex);
+  const auto token = m_destroy_callback_next_token++;
+  m_destroy_callback_and_baton.emplace(
+      std::piecewise_construct, std::forward_as_tuple(token),
+      std::forward_as_tuple(destroy_callback, baton));
+  return token;
+}
+
+bool Debugger::RemoveDestroyCallback(
+    lldb_private::DebuggerDestroyCallbackToken token) {
+  std::lock_guard<std::recursive_mutex> guard(m_destroy_callback_mutex);
+  return m_destroy_callback_and_baton.erase(token);
 }
 
 static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,

lldb/test/API/python_api/debugger/TestDebuggerAPI.py

@@ -161,3 +161,69 @@ def foo(dbg_id):
         original_dbg_id = self.dbg.GetID()
         self.dbg.Destroy(self.dbg)
         self.assertEqual(destroy_dbg_id, original_dbg_id)
+
+    def test_AddDestroyCallback(self):
+        original_dbg_id = self.dbg.GetID()
+        called = []
+
+        def foo(dbg_id):
+            # Need nonlocal to modify closure variable.
+            nonlocal called
+            called += [('foo', dbg_id)]
+
+        def bar(dbg_id):
+            # Need nonlocal to modify closure variable.
+            nonlocal called
+            called += [('bar', dbg_id)]
+
+        token_foo = self.dbg.AddDestroyCallback(foo)
+        token_bar = self.dbg.AddDestroyCallback(bar)
+        self.dbg.Destroy(self.dbg)
+
+        # Should call both `foo()` and `bar()`. Order is undermined because
+        # of the `unordered_map` in the implementation.
+        self.assertTrue(('foo', original_dbg_id) in called)
+        self.assertTrue(('bar', original_dbg_id) in called)
+
+    def test_RemoveDestroyCallback(self):
+        original_dbg_id = self.dbg.GetID()
+        called = []
+
+        def foo(dbg_id):
+            # Need nonlocal to modify closure variable.
+            nonlocal called
+            called += [('foo', dbg_id)]
+
+        def bar(dbg_id):
+            # Need nonlocal to modify closure variable.
+            nonlocal called
+            called += [('bar', dbg_id)]
+
+        token_foo = self.dbg.AddDestroyCallback(foo)
+        token_bar = self.dbg.AddDestroyCallback(bar)
+        ret = self.dbg.RemoveDestroyCallback(token_foo)
+        self.dbg.Destroy(self.dbg)
+
+        # `Remove` should be successful
+        self.assertTrue(ret)
+        # Should only call `bar()`
+        self.assertEqual(called, [('bar', original_dbg_id)])
+
+    def test_RemoveDestroyCallback_invalid_token(self):
+        original_dbg_id = self.dbg.GetID()
+        magic_token_that_should_not_exist = 32413
+        called = []
+
+        def foo(dbg_id):
+            # Need nonlocal to modify closure variable.
+            nonlocal called
+            called += [('foo', dbg_id)]
+
+        token_foo = self.dbg.AddDestroyCallback(foo)
+        ret = self.dbg.RemoveDestroyCallback(magic_token_that_should_not_exist)
+        self.dbg.Destroy(self.dbg)
+
+        # `Remove` should be unsuccessful
+        self.assertFalse(ret)
+        # Should call `foo()`
+        self.assertEqual(called, [('foo', original_dbg_id)])