[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)
via lldb-commits
lldb-commits at lists.llvm.org
Mon May 13 14:48:25 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 01/16] 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 418c2403d020f..20884f954ec7d 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 19b3cf3bbf46b..0ebdf2b0a0f97 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 02/16] 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 20884f954ec7d..af025219b0bc1 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 03/16] 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 af025219b0bc1..5b3e433f09c68 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 0ebdf2b0a0f97..159913642f253 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 04/16] 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 cf5409a12a056..2501f8efb45ad 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 fbcf30e67fc1c..754e3d27024a9 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 522de2466012e..46c62ea222a2c 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 05/16] 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 46c62ea222a2c..cbcf60587a89a 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)
>From b295e0d961938a218ca7f743067390f269c0603b Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Wed, 24 Apr 2024 14:54:20 -0700
Subject: [PATCH 06/16] Improve comment and Fix code format
---
lldb/include/lldb/Core/Debugger.h | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 5b3e433f09c68..0669f03116455 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -568,20 +568,19 @@ 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.
+ /// Add a callback for when the debugger is destroyed. Multiple callbacks
+ /// can be added by calling this function multiple times.
void
AddDestroyCallback(lldb_private::DebuggerDestroyCallback destroy_callback,
void *baton);
- /// Clear the list of callbacks, then add the callback.
+ /// Clear all previously added callbacks and only add the given one.
void
SetDestroyCallback(lldb_private::DebuggerDestroyCallback destroy_callback,
void *baton);
- /// Clear the list of callbacks.
- void
- ClearDestroyCallback();
+ /// Clear all previously added 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
>From 4ac92deaa1baa2e5ccc9359d12a760c32720f5e4 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Thu, 25 Apr 2024 13:30:21 -0700
Subject: [PATCH 07/16] Thread-safe Add/Remove with tokens
---
lldb/include/lldb/API/SBDebugger.h | 6 +--
lldb/include/lldb/API/SBDefines.h | 2 +
lldb/include/lldb/Core/Debugger.h | 25 +++++----
lldb/include/lldb/lldb-private-types.h | 1 +
lldb/source/API/SBDebugger.cpp | 11 ++--
lldb/source/Core/Debugger.cpp | 33 ++++++++----
.../python_api/debugger/TestDebuggerAPI.py | 54 ++++++++++++++-----
7 files changed, 92 insertions(+), 40 deletions(-)
diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index 2501f8efb45ad..5d1e4deb6424d 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -321,13 +321,13 @@ class LLDB_API SBDebugger {
void SetLoggingCallback(lldb::LogOutputCallback log_callback, void *baton);
- void AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
+ lldb::SBDebuggerDestroyCallbackToken AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
void *baton);
- void SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
+ lldb::SBDebuggerDestroyCallbackToken SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
void *baton);
- void ClearDestroyCallback();
+ bool RemoveDestroyCallback(lldb::SBDebuggerDestroyCallbackToken);
#ifndef SWIG
LLDB_DEPRECATED_FIXME("Use DispatchInput(const void *, size_t)",
diff --git a/lldb/include/lldb/API/SBDefines.h b/lldb/include/lldb/API/SBDefines.h
index 1181920677b46..f1f1e0bfd9a84 100644
--- a/lldb/include/lldb/API/SBDefines.h
+++ b/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);
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 0669f03116455..93ad5abde8c02 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -14,6 +14,7 @@
#include <memory>
#include <optional>
#include <vector>
+#include <unordered_map>
#include "lldb/Core/DebuggerEvents.h"
#include "lldb/Core/FormatEntity.h"
@@ -568,19 +569,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. Multiple callbacks
- /// can be added by calling this function multiple times.
- void
- AddDestroyCallback(lldb_private::DebuggerDestroyCallback destroy_callback,
- void *baton);
-
+ /// DEPRECATED. Use AddDestroyCallback and RemoveDestroyCallback instead.
/// Clear all previously added callbacks and only add the given one.
- void
+ lldb_private::DebuggerDestroyCallbackToken
SetDestroyCallback(lldb_private::DebuggerDestroyCallback destroy_callback,
void *baton);
- /// Clear all previously added callbacks.
- void ClearDestroyCallback();
+ /// 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
@@ -741,7 +744,9 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
lldb::TargetSP m_dummy_target_sp;
Diagnostics::CallbackID m_diagnostics_callback_id;
- std::vector<std::pair<lldb_private::DebuggerDestroyCallback, void *>>
+ 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
diff --git a/lldb/include/lldb/lldb-private-types.h b/lldb/include/lldb/lldb-private-types.h
index 7d301666df1a1..0d89dd34003d6 100644
--- a/lldb/include/lldb/lldb-private-types.h
+++ b/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)(
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 754e3d27024a9..2d7e496315cf5 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1686,29 +1686,32 @@ void SBDebugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
}
}
-void SBDebugger::AddDestroyCallback(
+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;
}
-void SBDebugger::SetDestroyCallback(
+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;
}
-void SBDebugger::ClearDestroyCallback() {
+bool SBDebugger::RemoveDestroyCallback(lldb::SBDebuggerDestroyCallbackToken token) {
LLDB_INSTRUMENT_VA(this);
if (m_opaque_sp) {
- return m_opaque_sp->ClearDestroyCallback();
+ return m_opaque_sp->RemoveDestroyCallback(token);
}
+ return false;
}
SBTrace
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 159913642f253..ea66db2c360d8 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -743,9 +743,12 @@ DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback,
}
void Debugger::HandleDestroyCallback() {
+ std::lock_guard<std::recursive_mutex> guard(m_destroy_callback_mutex);
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);
+ 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();
}
@@ -1426,19 +1429,27 @@ void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
std::make_shared<CallbackLogHandler>(log_callback, baton);
}
-void Debugger::AddDestroyCallback(
+lldb_private::DebuggerDestroyCallbackToken Debugger::SetDestroyCallback(
lldb_private::DebuggerDestroyCallback destroy_callback, void *baton) {
- m_destroy_callback_and_baton.emplace_back(destroy_callback, baton);
+ std::lock_guard<std::recursive_mutex> guard(m_destroy_callback_mutex);
+ m_destroy_callback_and_baton.clear();
+ return AddDestroyCallback(destroy_callback, baton);
}
-void Debugger::SetDestroyCallback(
+lldb_private::DebuggerDestroyCallbackToken Debugger::AddDestroyCallback(
lldb_private::DebuggerDestroyCallback destroy_callback, void *baton) {
- ClearDestroyCallback();
- AddDestroyCallback(destroy_callback, baton);
-}
-
-void Debugger::ClearDestroyCallback() {
- m_destroy_callback_and_baton.clear();
+ 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,
diff --git a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
index cbcf60587a89a..a8f3be2fe7136 100644
--- a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
+++ b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
@@ -176,24 +176,54 @@ def bar(dbg_id):
nonlocal called
called += [('bar', dbg_id)]
- self.dbg.AddDestroyCallback(foo)
- self.dbg.AddDestroyCallback(bar)
+ token_foo = self.dbg.AddDestroyCallback(foo)
+ token_bar = self.dbg.AddDestroyCallback(bar)
self.dbg.Destroy(self.dbg)
- # Should first call `foo()`, then `bar()`
- self.assertEqual(called, [('foo', original_dbg_id), ('bar', original_dbg_id)])
+ # 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_ClearDestroyCallback(self):
- destroy_dbg_id = None
+ def test_RemoveDestroyCallback(self):
+ 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', 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)]
- self.dbg.AddDestroyCallback(foo)
- self.dbg.ClearDestroyCallback()
+ token_foo = self.dbg.AddDestroyCallback(foo)
+ ret = self.dbg.RemoveDestroyCallback(magic_token_that_should_not_exist)
self.dbg.Destroy(self.dbg)
- # `foo()` should never be called
- self.assertEqual(destroy_dbg_id, None)
+ # `Remove` should be unsuccessful
+ self.assertFalse(ret)
+ # Should call `foo()`
+ self.assertEqual(called, [('foo', original_dbg_id)])
>From e5bf62059e4052806e1d4851d75c8c708090811c Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Thu, 25 Apr 2024 13:34:43 -0700
Subject: [PATCH 08/16] Fix format
---
lldb/include/lldb/API/SBDebugger.h | 10 ++++++----
lldb/include/lldb/Core/Debugger.h | 5 +++--
lldb/source/API/SBDebugger.cpp | 11 ++++++-----
lldb/source/Core/Debugger.cpp | 8 ++++----
4 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index 5d1e4deb6424d..a85016754f625 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -321,11 +321,13 @@ class LLDB_API SBDebugger {
void SetLoggingCallback(lldb::LogOutputCallback log_callback, void *baton);
- lldb::SBDebuggerDestroyCallbackToken AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
- void *baton);
+ lldb::SBDebuggerDestroyCallbackToken
+ AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
+ void *baton);
- lldb::SBDebuggerDestroyCallbackToken SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
- void *baton);
+ lldb::SBDebuggerDestroyCallbackToken
+ SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
+ void *baton);
bool RemoveDestroyCallback(lldb::SBDebuggerDestroyCallbackToken);
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 93ad5abde8c02..31d9556fb9ae7 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -13,8 +13,8 @@
#include <memory>
#include <optional>
-#include <vector>
#include <unordered_map>
+#include <vector>
#include "lldb/Core/DebuggerEvents.h"
#include "lldb/Core/FormatEntity.h"
@@ -746,7 +746,8 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
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 *>>
+ 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
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 2d7e496315cf5..b9e579b443c3c 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1686,8 +1686,8 @@ void SBDebugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
}
}
-lldb::SBDebuggerDestroyCallbackToken SBDebugger::AddDestroyCallback(
- 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(
@@ -1696,8 +1696,8 @@ lldb::SBDebuggerDestroyCallbackToken SBDebugger::AddDestroyCallback(
return -1;
}
-lldb::SBDebuggerDestroyCallbackToken SBDebugger::SetDestroyCallback(
- lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) {
+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(
@@ -1706,7 +1706,8 @@ lldb::SBDebuggerDestroyCallbackToken SBDebugger::SetDestroyCallback(
return -1;
}
-bool SBDebugger::RemoveDestroyCallback(lldb::SBDebuggerDestroyCallbackToken token) {
+bool SBDebugger::RemoveDestroyCallback(
+ lldb::SBDebuggerDestroyCallbackToken token) {
LLDB_INSTRUMENT_VA(this);
if (m_opaque_sp) {
return m_opaque_sp->RemoveDestroyCallback(token);
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index ea66db2c360d8..0724e1aed961d 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1441,13 +1441,13 @@ lldb_private::DebuggerDestroyCallbackToken Debugger::AddDestroyCallback(
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));
+ std::piecewise_construct, std::forward_as_tuple(token),
+ std::forward_as_tuple(destroy_callback, baton));
return token;
}
-bool Debugger::RemoveDestroyCallback(lldb_private::DebuggerDestroyCallbackToken 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);
}
>From 1206762edc4129a2b9a4571b2995ac737678a7d3 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Fri, 26 Apr 2024 12:39:03 -0700
Subject: [PATCH 09/16] Public token type and void return for Set
---
lldb/include/lldb/API/SBDebugger.h | 11 +++++------
lldb/include/lldb/API/SBDefines.h | 2 --
lldb/include/lldb/Core/Debugger.h | 15 +++++++++------
lldb/include/lldb/lldb-private-types.h | 1 -
lldb/include/lldb/lldb-types.h | 3 +++
lldb/source/API/SBDebugger.cpp | 21 +++++++++------------
lldb/source/Core/Debugger.cpp | 9 ++++-----
7 files changed, 30 insertions(+), 32 deletions(-)
diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index a85016754f625..5b50dd9a95e07 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -321,15 +321,14 @@ class LLDB_API SBDebugger {
void SetLoggingCallback(lldb::LogOutputCallback log_callback, void *baton);
- lldb::SBDebuggerDestroyCallbackToken
- AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
- void *baton);
+ void SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
+ void *baton);
- lldb::SBDebuggerDestroyCallbackToken
- SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
+ lldb::destroy_callback_token_t
+ AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
void *baton);
- bool RemoveDestroyCallback(lldb::SBDebuggerDestroyCallbackToken);
+ bool RemoveDestroyCallback(lldb::destroy_callback_token_t token);
#ifndef SWIG
LLDB_DEPRECATED_FIXME("Use DispatchInput(const void *, size_t)",
diff --git a/lldb/include/lldb/API/SBDefines.h b/lldb/include/lldb/API/SBDefines.h
index f1f1e0bfd9a84..1181920677b46 100644
--- a/lldb/include/lldb/API/SBDefines.h
+++ b/lldb/include/lldb/API/SBDefines.h
@@ -139,8 +139,6 @@ 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);
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 31d9556fb9ae7..58177b463b2e0 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -569,21 +569,24 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
static void ReportSymbolChange(const ModuleSpec &module_spec);
- /// DEPRECATED. Use AddDestroyCallback and RemoveDestroyCallback instead.
+ /// DEPRECATED: We used to only support one Destroy callback. Now that we
+ /// support Add and Remove, you should only remove Destroy callbacks that
+ /// you Add-ed. Use Add and Remove instead.
+ ///
/// Clear all previously added callbacks and only add the given one.
- lldb_private::DebuggerDestroyCallbackToken
+ void
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
+ lldb::destroy_callback_token_t
AddDestroyCallback(lldb_private::DebuggerDestroyCallback destroy_callback,
void *baton);
/// Remove the specified callback. Return true if successful.
- bool RemoveDestroyCallback(lldb_private::DebuggerDestroyCallbackToken token);
+ bool RemoveDestroyCallback(lldb::destroy_callback_token_t 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
@@ -745,8 +748,8 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
Diagnostics::CallbackID m_diagnostics_callback_id;
std::recursive_mutex m_destroy_callback_mutex;
- lldb_private::DebuggerDestroyCallbackToken m_destroy_callback_next_token = 0;
- std::unordered_map<lldb_private::DebuggerDestroyCallbackToken,
+ lldb::destroy_callback_token_t m_destroy_callback_next_token = 0;
+ std::unordered_map<lldb::destroy_callback_token_t,
std::pair<lldb_private::DebuggerDestroyCallback, void *>>
m_destroy_callback_and_baton;
diff --git a/lldb/include/lldb/lldb-private-types.h b/lldb/include/lldb/lldb-private-types.h
index 0d89dd34003d6..7d301666df1a1 100644
--- a/lldb/include/lldb/lldb-private-types.h
+++ b/lldb/include/lldb/lldb-private-types.h
@@ -121,7 +121,6 @@ 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)(
diff --git a/lldb/include/lldb/lldb-types.h b/lldb/include/lldb/lldb-types.h
index d60686e33142a..41afbc312ac6e 100644
--- a/lldb/include/lldb/lldb-types.h
+++ b/lldb/include/lldb/lldb-types.h
@@ -62,12 +62,15 @@ typedef void *thread_arg_t; // Host thread argument type
typedef void *thread_result_t; // Host thread result type
typedef void *(*thread_func_t)(void *); // Host thread function type
typedef int pipe_t; // Host pipe type
+typedef int destroy_callback_token_t; // Debugger destroy callback token type
#endif // _WIN32
#define LLDB_INVALID_PROCESS ((lldb::process_t)-1)
#define LLDB_INVALID_HOST_THREAD ((lldb::thread_t)NULL)
#define LLDB_INVALID_PIPE ((lldb::pipe_t)-1)
+#define LLDB_INVALID_DESTROY_CALLBACK_TOKEN \
+ ((lldb::destroy_callback_token_t) - 1)
typedef void (*LogOutputCallback)(const char *, void *baton);
typedef bool (*CommandOverrideCallback)(void *baton, const char **argv);
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index b9e579b443c3c..c1eb2aa5d45a6 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1686,28 +1686,25 @@ void SBDebugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
}
}
-lldb::SBDebuggerDestroyCallbackToken
-SBDebugger::AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback, void *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->AddDestroyCallback(
- destroy_callback, baton);
+ m_opaque_sp->SetDestroyCallback(destroy_callback, baton);
}
- return -1;
}
-lldb::SBDebuggerDestroyCallbackToken
-SBDebugger::SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) {
+lldb::destroy_callback_token_t
+SBDebugger::AddDestroyCallback(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 m_opaque_sp->AddDestroyCallback(destroy_callback, baton);
}
- return -1;
+ return LLDB_INVALID_DESTROY_CALLBACK_TOKEN;
}
-bool SBDebugger::RemoveDestroyCallback(
- lldb::SBDebuggerDestroyCallbackToken token) {
+bool SBDebugger::RemoveDestroyCallback(lldb::destroy_callback_token_t token) {
LLDB_INSTRUMENT_VA(this);
if (m_opaque_sp) {
return m_opaque_sp->RemoveDestroyCallback(token);
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 0724e1aed961d..6dc1119d1bedb 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1429,14 +1429,14 @@ void Debugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
std::make_shared<CallbackLogHandler>(log_callback, baton);
}
-lldb_private::DebuggerDestroyCallbackToken Debugger::SetDestroyCallback(
+void Debugger::SetDestroyCallback(
lldb_private::DebuggerDestroyCallback destroy_callback, void *baton) {
std::lock_guard<std::recursive_mutex> guard(m_destroy_callback_mutex);
m_destroy_callback_and_baton.clear();
- return AddDestroyCallback(destroy_callback, baton);
+ AddDestroyCallback(destroy_callback, baton);
}
-lldb_private::DebuggerDestroyCallbackToken Debugger::AddDestroyCallback(
+lldb::destroy_callback_token_t 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++;
@@ -1446,8 +1446,7 @@ lldb_private::DebuggerDestroyCallbackToken Debugger::AddDestroyCallback(
return token;
}
-bool Debugger::RemoveDestroyCallback(
- lldb_private::DebuggerDestroyCallbackToken token) {
+bool Debugger::RemoveDestroyCallback(lldb::destroy_callback_token_t token) {
std::lock_guard<std::recursive_mutex> guard(m_destroy_callback_mutex);
return m_destroy_callback_and_baton.erase(token);
}
>From d7e8b8da44db0a9aa95895b6e74e01a57cda49f1 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Fri, 26 Apr 2024 13:44:25 -0700
Subject: [PATCH 10/16] Handle Add/Remove during Destroy
---
lldb/source/Core/Debugger.cpp | 16 ++++--
.../python_api/debugger/TestDebuggerAPI.py | 52 +++++++++++++++++++
2 files changed, 63 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 6dc1119d1bedb..6a1e8e3782b61 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -745,12 +745,18 @@ DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback,
void Debugger::HandleDestroyCallback() {
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);
+ // In case one destroy callback adds or removes other destroy callbacks
+ // which aren't taken care of in the same inner loop.
+ while (m_destroy_callback_and_baton.size()) {
+ auto iter = m_destroy_callback_and_baton.begin();
+ while (iter != m_destroy_callback_and_baton.end()) {
+ // Invoke the callback and remove the entry from the map
+ const auto &callback = iter->second.first;
+ const auto &baton = iter->second.second;
+ callback(user_id, baton);
+ iter = m_destroy_callback_and_baton.erase(iter);
+ }
}
- m_destroy_callback_and_baton.clear();
}
void Debugger::Destroy(DebuggerSP &debugger_sp) {
diff --git a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
index a8f3be2fe7136..6fdc983758856 100644
--- a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
+++ b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
@@ -227,3 +227,55 @@ def foo(dbg_id):
self.assertFalse(ret)
# Should call `foo()`
self.assertEqual(called, [('foo', original_dbg_id)])
+
+ def test_HandleDestroyCallback(self):
+ """
+ Validates:
+ 1. Add and Remove can function during debugger destroy.
+ 2. HandleDestroyCallback can invoke all callbacks.
+ """
+ original_dbg_id = self.dbg.GetID()
+ events = {}
+
+ def foo(dbg_id):
+ # Need nonlocal to modify closure variable.
+ nonlocal events
+ events['foo called'] = dbg_id
+
+ def bar(dbg_id):
+ # Don't log bar()'s invokation because it may or may not be called
+ # based on its position in the container relative to remove_bar
+ pass
+
+ def add_foo(dbg_id):
+ # Need nonlocal to modify closure variable.
+ nonlocal events
+ events['add_foo called'] = dbg_id
+ events['foo token'] = self.dbg.AddDestroyCallback(foo)
+
+ def remove_bar(dbg_id):
+ # Need nonlocal to modify closure variable.
+ nonlocal events
+ events['remove_bar called'] = dbg_id
+ events['remove bar ret'] = self.dbg.RemoveDestroyCallback(events['bar token'])
+
+ # Setup
+ events['add_foo token'] = self.dbg.AddDestroyCallback(add_foo)
+ events['bar token'] = self.dbg.AddDestroyCallback(bar)
+ events['remove_bar token'] = self.dbg.AddDestroyCallback(remove_bar)
+ # Destroy
+ self.dbg.Destroy(self.dbg)
+
+ self.assertEqual(events, {
+ # Setup
+ 'add_foo token': 0, # add_foo should be added
+ 'bar token': 1, # bar should be added
+ 'remove_bar token': 2, # remove_bar should be added
+ # Destroy
+ 'add_foo called': original_dbg_id, # add_foo should be called
+ 'foo token': 3, # foo should be added
+ # bar may or may not be called
+ 'remove_bar called': original_dbg_id, # remove_bar should be called
+ 'remove bar ret': True, # remove_bar should succeed
+ 'foo called': original_dbg_id, # foo should be called
+ })
>From 192b00e03d6f856c74358c0b441549a34e0638ac Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Fri, 26 Apr 2024 13:50:31 -0700
Subject: [PATCH 11/16] Add comments
---
lldb/include/lldb/API/SBDebugger.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index 5b50dd9a95e07..49e7382e9c61f 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -321,13 +321,22 @@ class LLDB_API SBDebugger {
void SetLoggingCallback(lldb::LogOutputCallback log_callback, void *baton);
+ /// DEPRECATED: We used to only support one Destroy callback. Now that we
+ /// support Add and Remove, you should only remove Destroy callbacks that
+ /// you Add-ed. Use Add and Remove instead.
+ ///
+ /// Clear all previously added callbacks and only add the given one.
void SetDestroyCallback(lldb::SBDebuggerDestroyCallback 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::destroy_callback_token_t
AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
void *baton);
+ /// Remove the specified callback. Return true if successful.
bool RemoveDestroyCallback(lldb::destroy_callback_token_t token);
#ifndef SWIG
>From 1e02efc0df33906904e7545190fc78e6146278bd Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Fri, 26 Apr 2024 14:15:21 -0700
Subject: [PATCH 12/16] Add LLDB_DEPRECATED_FIXME
---
lldb/include/lldb/API/SBDebugger.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index 49e7382e9c61f..58cf8deea4bec 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -326,6 +326,8 @@ class LLDB_API SBDebugger {
/// you Add-ed. Use Add and Remove instead.
///
/// Clear all previously added callbacks and only add the given one.
+ LLDB_DEPRECATED_FIXME("Use AddDestroyCallback and RemoveDestroyCallback",
+ "AddDestroyCallback")
void SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
void *baton);
>From 7ec134644bc71f2722fbbebba26a05e87f6ecfff Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Mon, 29 Apr 2024 14:05:54 -0700
Subject: [PATCH 13/16] Simplify loop logic and update comments
---
lldb/source/Core/Debugger.cpp | 23 +++++++++++--------
.../python_api/debugger/TestDebuggerAPI.py | 5 ++--
2 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 6a1e8e3782b61..ce098c7d4a0e3 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -745,17 +745,22 @@ DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback,
void Debugger::HandleDestroyCallback() {
std::lock_guard<std::recursive_mutex> guard(m_destroy_callback_mutex);
const lldb::user_id_t user_id = GetID();
- // In case one destroy callback adds or removes other destroy callbacks
- // which aren't taken care of in the same inner loop.
+ // This loop handles the case where callbacks are added/removed by existing
+ // callbacks during the loop, as the following:
+ // - Added callbacks will always be invoked.
+ // - Removed callbacks will never be invoked. That is *unless* the loop
+ // happens to invoke the said callbacks first, before they get removed.
+ // In this case, the callbacks gets invoked, and the removal return false.
+ //
+ // In the removal case, because the order of the container (`unordered_map`)
+ // is random, it's wise to not depend on the order and instead implement
+ // logic inside the callbacks to decide if their work should be skipped.
while (m_destroy_callback_and_baton.size()) {
auto iter = m_destroy_callback_and_baton.begin();
- while (iter != m_destroy_callback_and_baton.end()) {
- // Invoke the callback and remove the entry from the map
- const auto &callback = iter->second.first;
- const auto &baton = iter->second.second;
- callback(user_id, baton);
- iter = m_destroy_callback_and_baton.erase(iter);
- }
+ const auto &callback = iter->second.first;
+ const auto &baton = iter->second.second;
+ callback(user_id, baton);
+ m_destroy_callback_and_baton.erase(iter);
}
}
diff --git a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
index 6fdc983758856..ab8c3cc194a59 100644
--- a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
+++ b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
@@ -243,8 +243,9 @@ def foo(dbg_id):
events['foo called'] = dbg_id
def bar(dbg_id):
- # Don't log bar()'s invokation because it may or may not be called
- # based on its position in the container relative to remove_bar
+ # Don't log/validate bar's invocation because it may or may not be
+ # called based on its position in the container relative to
+ # remove_bar.
pass
def add_foo(dbg_id):
>From 54f3d8d71ea47d42e49253d6bd197783f9f42c79 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Thu, 2 May 2024 22:36:04 -0700
Subject: [PATCH 14/16] Use SmallDenseMap, and coding standard fixes
---
lldb/include/lldb/API/SBDebugger.h | 4 ----
lldb/include/lldb/Core/Debugger.h | 8 ++++----
lldb/source/API/SBDebugger.cpp | 6 +++++-
lldb/source/Core/Debugger.cpp | 23 +++++++++++++----------
4 files changed, 22 insertions(+), 19 deletions(-)
diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index 58cf8deea4bec..9157adca5010d 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -321,10 +321,6 @@ class LLDB_API SBDebugger {
void SetLoggingCallback(lldb::LogOutputCallback log_callback, void *baton);
- /// DEPRECATED: We used to only support one Destroy callback. Now that we
- /// support Add and Remove, you should only remove Destroy callbacks that
- /// you Add-ed. Use Add and Remove instead.
- ///
/// Clear all previously added callbacks and only add the given one.
LLDB_DEPRECATED_FIXME("Use AddDestroyCallback and RemoveDestroyCallback",
"AddDestroyCallback")
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 58177b463b2e0..dfddd0180706d 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -570,8 +570,8 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
static void ReportSymbolChange(const ModuleSpec &module_spec);
/// DEPRECATED: We used to only support one Destroy callback. Now that we
- /// support Add and Remove, you should only remove Destroy callbacks that
- /// you Add-ed. Use Add and Remove instead.
+ /// support Add and Remove, you should only remove callbacks that you added.
+ /// Use Add and Remove instead.
///
/// Clear all previously added callbacks and only add the given one.
void
@@ -749,8 +749,8 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
std::recursive_mutex m_destroy_callback_mutex;
lldb::destroy_callback_token_t m_destroy_callback_next_token = 0;
- std::unordered_map<lldb::destroy_callback_token_t,
- std::pair<lldb_private::DebuggerDestroyCallback, void *>>
+ llvm::SmallDenseMap<lldb::destroy_callback_token_t,
+ std::pair<lldb_private::DebuggerDestroyCallback, void *>>
m_destroy_callback_and_baton;
uint32_t m_interrupt_requested = 0; ///< Tracks interrupt requests
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index c1eb2aa5d45a6..c6040ed24829a 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1689,6 +1689,7 @@ void SBDebugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
void SBDebugger::SetDestroyCallback(
lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) {
LLDB_INSTRUMENT_VA(this, destroy_callback, baton);
+
if (m_opaque_sp) {
m_opaque_sp->SetDestroyCallback(destroy_callback, baton);
}
@@ -1698,6 +1699,7 @@ lldb::destroy_callback_token_t
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);
}
@@ -1705,7 +1707,8 @@ SBDebugger::AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
}
bool SBDebugger::RemoveDestroyCallback(lldb::destroy_callback_token_t token) {
- LLDB_INSTRUMENT_VA(this);
+ LLDB_INSTRUMENT_VA(this, token);
+
if (m_opaque_sp) {
return m_opaque_sp->RemoveDestroyCallback(token);
}
@@ -1716,6 +1719,7 @@ 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);
}
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index ce098c7d4a0e3..c67d82325d147 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -752,15 +752,19 @@ void Debugger::HandleDestroyCallback() {
// happens to invoke the said callbacks first, before they get removed.
// In this case, the callbacks gets invoked, and the removal return false.
//
- // In the removal case, because the order of the container (`unordered_map`)
- // is random, it's wise to not depend on the order and instead implement
- // logic inside the callbacks to decide if their work should be skipped.
+ // In the removal case, because the order of the container is random, it's
+ // wise to not depend on the order and instead implement logic inside the
+ // callbacks to decide if their work should be skipped.
while (m_destroy_callback_and_baton.size()) {
auto iter = m_destroy_callback_and_baton.begin();
- const auto &callback = iter->second.first;
- const auto &baton = iter->second.second;
+ const lldb::destroy_callback_token_t &token = iter->first;
+ const lldb_private::DebuggerDestroyCallback &callback = iter->second.first;
+ void *const &baton = iter->second.second;
callback(user_id, baton);
- m_destroy_callback_and_baton.erase(iter);
+ // Using `token` to erase, because elements may have been added/removed, and
+ // that will cause error "invalid iterator access!" if `iter` is used
+ // instead.
+ m_destroy_callback_and_baton.erase(token);
}
}
@@ -1450,10 +1454,9 @@ void Debugger::SetDestroyCallback(
lldb::destroy_callback_token_t 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));
+ const lldb::destroy_callback_token_t token = m_destroy_callback_next_token++;
+ m_destroy_callback_and_baton.try_emplace(
+ token, std::forward_as_tuple(destroy_callback, baton));
return token;
}
>From 496dec68c0bdf090d823dc15d1c1f2ccd086fa7d Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Mon, 13 May 2024 14:45:29 -0700
Subject: [PATCH 15/16] Address comments from Greg and Jeffrey
---
lldb/include/lldb/Core/Debugger.h | 1 -
lldb/source/API/SBDebugger.cpp | 8 ++++----
lldb/source/Core/Debugger.cpp | 8 ++++----
lldb/test/API/python_api/debugger/TestDebuggerAPI.py | 4 ++--
4 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index dfddd0180706d..3af00b1e572b7 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -13,7 +13,6 @@
#include <memory>
#include <optional>
-#include <unordered_map>
#include <vector>
#include "lldb/Core/DebuggerEvents.h"
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index c6040ed24829a..e044a00c18d77 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1700,18 +1700,18 @@ SBDebugger::AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
void *baton) {
LLDB_INSTRUMENT_VA(this, destroy_callback, baton);
- if (m_opaque_sp) {
+ if (m_opaque_sp)
return m_opaque_sp->AddDestroyCallback(destroy_callback, baton);
- }
+
return LLDB_INVALID_DESTROY_CALLBACK_TOKEN;
}
bool SBDebugger::RemoveDestroyCallback(lldb::destroy_callback_token_t token) {
LLDB_INSTRUMENT_VA(this, token);
- if (m_opaque_sp) {
+ if (m_opaque_sp)
return m_opaque_sp->RemoveDestroyCallback(token);
- }
+
return false;
}
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index c67d82325d147..a6688ec6a68cf 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -755,11 +755,11 @@ void Debugger::HandleDestroyCallback() {
// In the removal case, because the order of the container is random, it's
// wise to not depend on the order and instead implement logic inside the
// callbacks to decide if their work should be skipped.
- while (m_destroy_callback_and_baton.size()) {
+ while (!m_destroy_callback_and_baton.empty()) {
auto iter = m_destroy_callback_and_baton.begin();
- const lldb::destroy_callback_token_t &token = iter->first;
- const lldb_private::DebuggerDestroyCallback &callback = iter->second.first;
- void *const &baton = iter->second.second;
+ const lldb::destroy_callback_token_t token = iter->first;
+ const lldb_private::DebuggerDestroyCallback callback = iter->second.first;
+ void *const baton = iter->second.second;
callback(user_id, baton);
// Using `token` to erase, because elements may have been added/removed, and
// that will cause error "invalid iterator access!" if `iter` is used
diff --git a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
index ab8c3cc194a59..8e163fdf9d046 100644
--- a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
+++ b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
@@ -182,8 +182,8 @@ def bar(dbg_id):
# 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)
+ self.assertIn(('foo', original_dbg_id), called)
+ self.assertIn(('bar', original_dbg_id), called)
def test_RemoveDestroyCallback(self):
original_dbg_id = self.dbg.GetID()
>From 7ebac23a1d1fc53daf9457f0ba7cc13aafcd53fa Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Mon, 13 May 2024 14:48:08 -0700
Subject: [PATCH 16/16] Attempt to revert whitespace changes in other code
---
lldb/include/lldb/API/SBDebugger.h | 2 +-
lldb/source/API/SBDebugger.cpp | 11 +++++------
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index 9157adca5010d..f04b5905aa228 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();
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index e044a00c18d77..88c0437704376 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1719,26 +1719,25 @@ 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);
}
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;
More information about the lldb-commits
mailing list