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 <roy...@meta.com>
Date: Tue, 23 Apr 2024 18:10:21 -0700
Subject: [PATCH 1/8] 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 <roy...@meta.com>
Date: Tue, 23 Apr 2024 21:05:58 -0700
Subject: [PATCH 2/8] 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 <roy...@meta.com>
Date: Wed, 24 Apr 2024 11:55:16 -0700
Subject: [PATCH 3/8] 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 <roy...@meta.com>
Date: Wed, 24 Apr 2024 14:18:25 -0700
Subject: [PATCH 4/8] 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 <roy...@meta.com>
Date: Wed, 24 Apr 2024 14:47:52 -0700
Subject: [PATCH 5/8] 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)
>From b295e0d961938a218ca7f743067390f269c0603b Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Wed, 24 Apr 2024 14:54:20 -0700
Subject: [PATCH 6/8] 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 5b3e433f09c68e..0669f031164558 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 <roy...@meta.com>
Date: Thu, 25 Apr 2024 13:30:21 -0700
Subject: [PATCH 7/8] 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 2501f8efb45ad5..5d1e4deb6424d0 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 1181920677b46f..f1f1e0bfd9a84f 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 0669f031164558..93ad5abde8c02d 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 7d301666df1a17..0d89dd34003d6c 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 754e3d27024a9f..2d7e496315cf51 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 159913642f253e..ea66db2c360d8c 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 cbcf60587a89aa..a8f3be2fe71362 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 <roy...@meta.com>
Date: Thu, 25 Apr 2024 13:34:43 -0700
Subject: [PATCH 8/8] 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 5d1e4deb6424d0..a85016754f6256 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 93ad5abde8c02d..31d9556fb9ae7e 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 2d7e496315cf51..b9e579b443c3c3 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 ea66db2c360d8c..0724e1aed961d5 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);
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
