Add a new SBDebugger::SetDestroyCallback() API

[yinghuitan]

Summary

Adding a new SBDebugger::SetDestroyCallback() API.
This API can be used by any client to query for statistics/metrics before
exiting debug sessions.

[clayborg]

If I am reading the code for this patch correctly, we need the BatonSP stuff because we have differing callback bytes for public vs private APIs. If we switch to using a common callback type of:

typedef void (*DebuggerDestroyCallback)(lldb::user_id_t &debugger_id, void *baton);

(we can define this both internally and leave the current "SBDebuggerDestroyCallback" in SBDefines.h alone), then we can avoid having to add any of the BatonSP stuff and just store a "void * m_destroy_callback_baton;" in Debugger.h.

lldb/include/lldb/Core/Debugger.h

600–601

Why do we need the fancy BatonSP stuff here? Can't we just store this as a void *? Or is the fancy baton stuff required for the python for some reason? I couldn't see any reason by reading the code quickly, please correct me if so.

lldb/include/lldb/lldb-types.h

71–72 ↗

(On Diff #495599)

This can't be in the public lldb-types.h header file as no on will be able to use it since it uses "lldb_private::Debugger". If this is to be in the public header files, then this needs to change to be one of:

typedef void (*DebuggerDestroyCallback)(lldb::user_id_t &debugger_id, void *baton);
typedef void (*DebuggerDestroyCallback)(lldb::SBDebugger &debugger, void *baton);

The "debugger_id" is the easier one to do since we need to make this work with python so that python users can install a callback from python and have it all work.

This can however exist in the lldb-private-types.h which is not part of the public API, so it can be moved there and then it won't cause any issues. I realize there are other typedefs in here than mention lldb_private stuff, but they shouldn't be here.

[yinghuitan]

Address review comments by using the same type for internal and
public callbacks.

[clayborg]

Just change HandleDestroryCallback to a member function and this is good to go.

lldb/source/Core/Debugger.cpp

688–694

This should be a member function instead of a static function that takes a debugger_sp an argument.

699

[yinghuitan]

Address review feedback.

lldb/bindings/python/python-typemaps.swig

%typemap(in) (lldb::LogOutputCallback log_callback, void *baton) {
   $2 = $input;
 }
 
 %typemap(typecheck) (lldb::LogOutputCallback log_callback, void *baton) {
   $1 = $input == Py_None;
   $1 = $1 || PyCallable_Check(reinterpret_cast<PyObject *>($input));
 }
 
+// For lldb::SBDebuggerDestroyCallback
+%typemap(in) (lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) {
+  if (!($input == Py_None ||
+        PyCallable_Check(reinterpret_cast<PyObject *>($input)))) {
+    PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+    SWIG_fail;
+  }
+
+  // FIXME (filcab): We can't currently check if our callback is already
+  // LLDBSwigPythonCallPythonSBDebuggerTerminateCallback (to DECREF the previous
+  // baton) nor can we just remove all traces of a callback, if we want to
+  // revert to a file logging mechanism.
+
+  // Don't lose the callback reference
+  Py_INCREF($input);
+  $1 = LLDBSwigPythonCallPythonSBDebuggerTerminateCallback;
+  $2 = $input;
+}
+
+%typemap(typecheck) (lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) {
+  $1 = $input == Py_None;
+  $1 = $1 || PyCallable_Check(reinterpret_cast<PyObject *>($input));
+}
 
 %typemap(in) lldb::FileSP {
   PythonFile py_file(PyRefType::Borrowed, $input);
   if (!py_file) {
     PyErr_SetString(PyExc_TypeError, "not a file");
     SWIG_fail;
   }
   auto sp = unwrapOrSetPythonException(py_file.ConvertToFile());

lldb/bindings/python/python-wrapper.swig

static void LLDBSwigPythonCallPythonLogOutputCallback(const char *str,
   if (baton != Py_None) {
     SWIG_PYTHON_THREAD_BEGIN_BLOCK;
     PyObject *result = PyObject_CallFunction(
         reinterpret_cast<PyObject *>(baton), const_cast<char *>("s"), str);
     Py_XDECREF(result);
     SWIG_PYTHON_THREAD_END_BLOCK;
   }
 }
+
+// For DebuggerTerminateCallback functions
+static void LLDBSwigPythonCallPythonSBDebuggerTerminateCallback(lldb::user_id_t debugger_id,
+                                                      void *baton) {
+  if (baton != Py_None) {
+    SWIG_PYTHON_THREAD_BEGIN_BLOCK;
+    PyObject *result = PyObject_CallFunction(
+        reinterpret_cast<PyObject *>(baton), const_cast<char *>("l"), debugger_id);
+    Py_XDECREF(result);
+    SWIG_PYTHON_THREAD_END_BLOCK;
+  }
+}
 %}

lldb/include/lldb/API/SBDebugger.h

#endif
   static bool StateIsRunningState(lldb::StateType state);
 
   static bool StateIsStoppedState(lldb::StateType state);
 
   bool EnableLog(const char *channel, const char **categories);
 
   void SetLoggingCallback(lldb::LogOutputCallback log_callback, void *baton);
 
+  void SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
+                          void *baton);
+
   // DEPRECATED
 #ifndef SWIG
   void DispatchInput(void *baton, const void *data, size_t data_len);
 #endif
 
   void DispatchInput(const void *data, size_t data_len);
 
   void DispatchInputInterrupt();

lldb/include/lldb/API/SBDefines.h

 class LLDB_API SBValueList;
 class LLDB_API SBVariablesOptions;
 class LLDB_API SBWatchpoint;
 class LLDB_API SBUnixSignals;
 
 typedef bool (*SBBreakpointHitCallback)(void *baton, SBProcess &process,
                                         SBThread &thread,
                                         lldb::SBBreakpointLocation &location);
+
+typedef void (*SBDebuggerDestroyCallback)(lldb::user_id_t debugger_id,
+                                          void *baton);
+
 typedef void *ScriptedObject;
 }
 
 #endif // LLDB_API_SBDEFINES_H

lldb/include/lldb/Core/Debugger.h

public:
   ///   ensure the given info is only logged once.
   static void
   ReportInfo(std::string message,
              std::optional<lldb::user_id_t> debugger_id = std::nullopt,
              std::once_flag *once = nullptr);
 
   static void ReportSymbolChange(const ModuleSpec &module_spec);
 
+  void
+  SetDestroyCallback(lldb_private::DebuggerDestroyCallback destroy_callback,
+                     void *baton);
+
 protected:
   friend class CommandInterpreter;
   friend class REPL;
   friend class Progress;
 
   /// Report progress events.
   ///
   /// Progress events will be delivered to any debuggers that have listeners
static void ReportProgress(uint64_t progress_id, std::string title,
                              uint64_t total,
                              std::optional<lldb::user_id_t> debugger_id);
 
   static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
                                    std::string message,
                                    std::optional<lldb::user_id_t> debugger_id,
                                    std::once_flag *once);
 
+  void HandleDestroyCallback();
+
   void PrintProgress(const ProgressEventData &data);
 
   bool StartEventHandlerThread();
 
   void StopEventHandlerThread();
 
   void PushIOHandler(const lldb::IOHandlerSP &reader_sp,
                      bool cancel_top_handler = true);
protected:
   HostThread m_event_handler_thread;
   HostThread m_io_handler_thread;
   Broadcaster m_sync_broadcaster; ///< Private debugger synchronization.
   Broadcaster m_broadcaster;      ///< Public Debugger event broadcaster.
   lldb::ListenerSP m_forward_listener_sp;
   llvm::once_flag m_clear_once;
   lldb::TargetSP m_dummy_target_sp;
 
+  lldb_private::DebuggerDestroyCallback m_destroy_callback = nullptr;
+  void *m_destroy_callback_baton = nullptr;
+
   // Events for m_sync_broadcaster
   enum {
     eBroadcastBitEventThreadIsListening = (1 << 0),
   };
 
 private:
   // Use Debugger::CreateInstance() to get a shared pointer to a new debugger
   // object

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

 typedef struct type256 { uint64_t x[4]; } type256;
 
 /// Functor that returns a ValueObjectSP for a variable given its name
 /// and the StackFrame of interest. Used primarily in the Materializer
 /// to refetch a ValueObject when the ExecutionContextScope changes.
 using ValueObjectProviderTy =
     std::function<lldb::ValueObjectSP(ConstString, StackFrame *)>;
 
+typedef void (*DebuggerDestroyCallback)(lldb::user_id_t debugger_id,
+                                        void *baton);
 } // namespace lldb_private
 
 #endif // #if defined(__cplusplus)
 
 #endif // LLDB_LLDB_PRIVATE_TYPES_H

lldb/source/API/SBDebugger.cpp

void SBDebugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
                                     void *baton) {
   LLDB_INSTRUMENT_VA(this, log_callback, baton);
 
   if (m_opaque_sp) {
     return m_opaque_sp->SetLoggingCallback(log_callback, baton);
   }
 }
 
+void 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);
+  }
+}
+
 SBTrace
 SBDebugger::LoadTraceFromFile(SBError &error,
                               const SBFileSpec &trace_description_file) {
   LLDB_INSTRUMENT_VA(this, error, trace_description_file);
   return SBTrace::LoadTraceFromFile(error, *this, trace_description_file);
 }

lldb/source/Core/Debugger.cpp

void Debugger::Initialize(LoadPluginCallbackType load_plugin_callback) {
   g_thread_pool = new llvm::ThreadPool(llvm::optimal_concurrency());
   g_load_plugin_callback = load_plugin_callback;
 }
 
 void Debugger::Terminate() {
   assert(g_debugger_list_ptr &&
          "Debugger::Terminate called without a matching Debugger::Initialize!");
 
+  if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
+    std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr);
+    for (const auto &debugger : *g_debugger_list_ptr)
+      debugger->HandleDestroyCallback();
+  }
+
   if (g_thread_pool) {
     // The destructor will wait for all the threads to complete.
     delete g_thread_pool;
   }
 
   if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
     // Clear our global list of debugger objects
     {
DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback,
   if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
     std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr);
     g_debugger_list_ptr->push_back(debugger_sp);
   }
   debugger_sp->InstanceInitialize();
   return debugger_sp;
 }
 
+void Debugger::HandleDestroyCallback() {
+  if (m_destroy_callback) {
+    m_destroy_callback(GetID(), m_destroy_callback_baton);
+    m_destroy_callback = nullptr;
+  }
+}
+
 void Debugger::Destroy(DebuggerSP &debugger_sp) {
   if (!debugger_sp)
     return;
 
+  debugger_sp->HandleDestroyCallback();
   CommandInterpreter &cmd_interpreter = debugger_sp->GetCommandInterpreter();
 
   if (cmd_interpreter.GetSaveSessionOnQuit()) {
     CommandReturnObject result(debugger_sp->GetUseColor());
     cmd_interpreter.SaveTranscript(result);
     if (result.Succeeded())
       (*debugger_sp->GetAsyncOutputStream()) << result.GetOutputData() << '\n';
     else
void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
                                   void *baton) {
   // For simplicity's sake, I am not going to deal with how to close down any
   // open logging streams, I just redirect everything from here on out to the
   // callback.
   m_callback_handler_sp =
       std::make_shared<CallbackLogHandler>(log_callback, baton);
 }
 
+void Debugger::SetDestroyCallback(
+    lldb_private::DebuggerDestroyCallback destroy_callback, void *baton) {
+  m_destroy_callback = destroy_callback;
+  m_destroy_callback_baton = baton;
+}
+
 static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
                                   std::string title, std::string details,
                                   uint64_t completed, uint64_t total,
                                   bool is_debugger_specific) {
   // Only deliver progress events if we have any progress listeners.
   const uint32_t event_type = Debugger::eBroadcastBitProgress;
   if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type))
     return;

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

def test_debugger_api_boundary_condition(self):
         self.dbg.SetInternalVariable(None, None, None)
         self.dbg.GetInternalVariableValue(None, None)
         # FIXME (filcab): We must first allow for the swig bindings to know if
         # a Python callback is set. (Check python-typemaps.swig)
         # self.dbg.SetLoggingCallback(None)
         self.dbg.SetPrompt(None)
         self.dbg.SetCurrentPlatform(None)
         self.dbg.SetCurrentPlatformSDKRoot(None)
-        
         fresh_dbg = lldb.SBDebugger()
         self.assertEquals(len(fresh_dbg), 0)
 
     def test_debugger_delete_invalid_target(self):
         """SBDebugger.DeleteTarget() should not crash LLDB given and invalid target."""
         target = lldb.SBTarget()
         self.assertFalse(target.IsValid())
         self.dbg.DeleteTarget(target)
def test_CreateTarget_arch(self):
         # Reuse a platform even if it is not currently selected.
         self.dbg.SetSelectedPlatform(fbsd)
         target2 = self.dbg.CreateTarget(exe, arch, None, False, error)
         self.assertSuccess(error)
         platform2 = target2.GetPlatform()
         self.assertEqual(platform2.GetName(), expected_platform)
         self.assertTrue(platform2.GetWorkingDirectory().endswith("bar"),
                 platform2.GetWorkingDirectory())
+
+    def test_SetDestroyCallback(self):
+        destroy_dbg_id = None
+        def foo(dbg_id):
+            # Need nonlocal to modify closure variable.
+            nonlocal destroy_dbg_id
+            destroy_dbg_id = dbg_id
+
+        self.dbg.SetDestroyCallback(foo)
+
+        original_dbg_id = self.dbg.GetID()
+        self.dbg.Destroy(self.dbg)
+        self.assertEqual(destroy_dbg_id,  original_dbg_id)