[Lldb-commits] [lldb] Allow multiple destroy callbacks in `SBDebugger::SetDestroyCallback()` (PR #89868)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Apr 24 14:48:14 PDT 2024
https://github.com/royitaqi updated https://github.com/llvm/llvm-project/pull/89868
>From 079a550481d4cdcb69ad01c376b5e1f0632a07d6 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Tue, 23 Apr 2024 18:10:21 -0700
Subject: [PATCH 1/5] Allow multiple destroy callbacks in
`SBDebugger::SetDestroyCallback()`
---
lldb/include/lldb/Core/Debugger.h | 4 ++--
lldb/source/Core/Debugger.cpp | 10 +++++-----
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 418c2403d020f4..20884f954ec7db 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -731,8 +731,8 @@ 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::vector<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;
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 19b3cf3bbf46b1..0ebdf2b0a0f970 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -743,10 +743,11 @@ 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;
+ const lldb::user_id_t user_id = GetID();
+ for (const auto &callback_and_baton : m_destroy_callback_and_baton) {
+ callback_and_baton.first(user_id, callback_and_baton.second);
}
+ m_destroy_callback_and_baton.clear();
}
void Debugger::Destroy(DebuggerSP &debugger_sp) {
@@ -1427,8 +1428,7 @@ void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
void Debugger::SetDestroyCallback(
lldb_private::DebuggerDestroyCallback destroy_callback, void *baton) {
- m_destroy_callback = destroy_callback;
- m_destroy_callback_baton = baton;
+ m_destroy_callback_and_baton.emplace_back(destroy_callback, baton);
}
static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
>From 771b52723be8d0ffecaf8f0818105cfdb82ba332 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Tue, 23 Apr 2024 21:05:58 -0700
Subject: [PATCH 2/5] Fix code format
---
lldb/include/lldb/Core/Debugger.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 20884f954ec7db..af025219b0bc12 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -732,7 +732,7 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
Diagnostics::CallbackID m_diagnostics_callback_id;
std::vector<std::pair<lldb_private::DebuggerDestroyCallback, void *>>
- m_destroy_callback_and_baton;
+ m_destroy_callback_and_baton;
uint32_t m_interrupt_requested = 0; ///< Tracks interrupt requests
std::mutex m_interrupt_mutex;
>From d1f13cad8a3789a994572459893b32a225ba3e1b Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Wed, 24 Apr 2024 11:55:16 -0700
Subject: [PATCH 3/5] Add `AddDestroyCallback()` and `ClearDestroyCallback()`
---
lldb/include/lldb/Core/Debugger.h | 11 +++++++++++
lldb/source/Core/Debugger.cpp | 12 +++++++++++-
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index af025219b0bc12..5b3e433f09c68e 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -568,10 +568,21 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
static void ReportSymbolChange(const ModuleSpec &module_spec);
+ /// Add a callback for when the debugger is destroyed. A list is maintained
+ /// internally.
+ void
+ AddDestroyCallback(lldb_private::DebuggerDestroyCallback destroy_callback,
+ void *baton);
+
+ /// Clear the list of callbacks, then add the callback.
void
SetDestroyCallback(lldb_private::DebuggerDestroyCallback destroy_callback,
void *baton);
+ /// Clear the list of callbacks.
+ void
+ ClearDestroyCallback();
+
/// 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.
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 0ebdf2b0a0f970..159913642f253e 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1426,11 +1426,21 @@ void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
std::make_shared<CallbackLogHandler>(log_callback, baton);
}
-void Debugger::SetDestroyCallback(
+void Debugger::AddDestroyCallback(
lldb_private::DebuggerDestroyCallback destroy_callback, void *baton) {
m_destroy_callback_and_baton.emplace_back(destroy_callback, baton);
}
+void Debugger::SetDestroyCallback(
+ lldb_private::DebuggerDestroyCallback destroy_callback, void *baton) {
+ ClearDestroyCallback();
+ AddDestroyCallback(destroy_callback, baton);
+}
+
+void Debugger::ClearDestroyCallback() {
+ m_destroy_callback_and_baton.clear();
+}
+
static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
std::string title, std::string details,
uint64_t completed, uint64_t total,
>From f536124f6f31ac72baf601fd438f21a04ac20691 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Wed, 24 Apr 2024 14:18:25 -0700
Subject: [PATCH 4/5] Reflect API changes in `SBDebugger` class and add test
---
lldb/include/lldb/API/SBDebugger.h | 7 +++-
lldb/source/API/SBDebugger.cpp | 26 +++++++++---
.../python_api/debugger/TestDebuggerAPI.py | 42 +++++++++++++++++++
3 files changed, 69 insertions(+), 6 deletions(-)
diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index cf5409a12a056a..2501f8efb45ad5 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/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,9 +321,14 @@ class LLDB_API SBDebugger {
void SetLoggingCallback(lldb::LogOutputCallback log_callback, void *baton);
+ void AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
+ void *baton);
+
void SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
void *baton);
+ void ClearDestroyCallback();
+
#ifndef SWIG
LLDB_DEPRECATED_FIXME("Use DispatchInput(const void *, size_t)",
"DispatchInput(const void *, size_t)")
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index fbcf30e67fc1cd..754e3d27024a9f 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1686,6 +1686,15 @@ void SBDebugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
}
}
+void 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);
+ }
+}
+
void SBDebugger::SetDestroyCallback(
lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) {
LLDB_INSTRUMENT_VA(this, destroy_callback, baton);
@@ -1695,6 +1704,13 @@ void SBDebugger::SetDestroyCallback(
}
}
+void SBDebugger::ClearDestroyCallback() {
+ LLDB_INSTRUMENT_VA(this);
+ if (m_opaque_sp) {
+ return m_opaque_sp->ClearDestroyCallback();
+ }
+}
+
SBTrace
SBDebugger::LoadTraceFromFile(SBError &error,
const SBFileSpec &trace_description_file) {
@@ -1704,20 +1720,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;
diff --git a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
index 522de2466012ed..46c62ea222a2c8 100644
--- a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
+++ b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
@@ -161,3 +161,45 @@ 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):
+ destroy_dbg_id = None
+ called = []
+
+ def foo(dbg_id):
+ # Need nonlocal to modify closure variable.
+ nonlocal destroy_dbg_id
+ destroy_dbg_id = dbg_id
+
+ nonlocal called
+ called += ['foo']
+
+ def bar(dbg_id):
+ # Need nonlocal to modify closure variable.
+ nonlocal destroy_dbg_id
+ destroy_dbg_id = dbg_id
+
+ nonlocal called
+ called += ['bar']
+
+ self.dbg.AddDestroyCallback(foo)
+ self.dbg.AddDestroyCallback(bar)
+
+ original_dbg_id = self.dbg.GetID()
+ self.dbg.Destroy(self.dbg)
+ self.assertEqual(destroy_dbg_id, original_dbg_id)
+ self.assertEqual(called, ['bar', 'foo'])
+
+ def test_ClearDestroyCallback(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.AddDestroyCallback(foo)
+ self.dbg.ClearDestroyCallback()
+
+ self.dbg.Destroy(self.dbg)
+ self.assertEqual(destroy_dbg_id, None)
>From 661dc1c27279d35ea9bf30a76fd1cbce045b37c7 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Wed, 24 Apr 2024 14:47:52 -0700
Subject: [PATCH 5/5] Update test
---
.../python_api/debugger/TestDebuggerAPI.py | 22 +++++++------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
index 46c62ea222a2c8..cbcf60587a89aa 100644
--- a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
+++ b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
@@ -163,32 +163,25 @@ def foo(dbg_id):
self.assertEqual(destroy_dbg_id, original_dbg_id)
def test_AddDestroyCallback(self):
- destroy_dbg_id = None
+ original_dbg_id = self.dbg.GetID()
called = []
def foo(dbg_id):
# Need nonlocal to modify closure variable.
- nonlocal destroy_dbg_id
- destroy_dbg_id = dbg_id
-
nonlocal called
- called += ['foo']
+ called += [('foo', dbg_id)]
def bar(dbg_id):
# Need nonlocal to modify closure variable.
- nonlocal destroy_dbg_id
- destroy_dbg_id = dbg_id
-
nonlocal called
- called += ['bar']
+ called += [('bar', dbg_id)]
self.dbg.AddDestroyCallback(foo)
self.dbg.AddDestroyCallback(bar)
-
- original_dbg_id = self.dbg.GetID()
self.dbg.Destroy(self.dbg)
- self.assertEqual(destroy_dbg_id, original_dbg_id)
- self.assertEqual(called, ['bar', 'foo'])
+
+ # Should first call `foo()`, then `bar()`
+ self.assertEqual(called, [('foo', original_dbg_id), ('bar', original_dbg_id)])
def test_ClearDestroyCallback(self):
destroy_dbg_id = None
@@ -200,6 +193,7 @@ def foo(dbg_id):
self.dbg.AddDestroyCallback(foo)
self.dbg.ClearDestroyCallback()
-
self.dbg.Destroy(self.dbg)
+
+ # `foo()` should never be called
self.assertEqual(destroy_dbg_id, None)
More information about the lldb-commits
mailing list