aboutsummaryrefslogtreecommitdiff
path: root/lldb/source/API/SBDebugger.cpp
diff options
context:
space:
mode:
authorroyitaqi <royitaqi@users.noreply.github.com>2024-05-20 15:51:42 -0700
committerGitHub <noreply@github.com>2024-05-20 15:51:42 -0700
commit9f62775038b9135709a2c3c7ea97c944278967a2 (patch)
tree06538df8f072cd1e575b5f87f406e17ddcd6e170 /lldb/source/API/SBDebugger.cpp
parente8dc8d614ada201e250fbf075241b2b6180943b5 (diff)
downloadllvm-9f62775038b9135709a2c3c7ea97c944278967a2.zip
llvm-9f62775038b9135709a2c3c7ea97c944278967a2.tar.gz
llvm-9f62775038b9135709a2c3c7ea97c944278967a2.tar.bz2
SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (#89868)
# Motivation Individual callers of `SBDebugger::SetDestroyCallback()` might think that they have registered their callback and expect it to be called when the debugger is destroyed. In reality, only the last caller survives, and all previous callers are forgotten, which might be a surprise to them. Worse, if this is called in a race condition, which callback survives is less predictable, which may case confusing behavior elsewhere. # This PR Allows multiple destroy callbacks to be registered and all called when the debugger is destroyed. **EDIT**: Adds two new APIs: `AddDestroyCallback()` and `ClearDestroyCallback()`. `SetDestroyCallback()` will first clear then add the given callback. Tests are added for the new APIs. ## Tests ``` bin/llvm-lit -sv ../external/llvm-project/lldb/test/API/python_api/debugger/TestDebuggerAPI.py ``` ## (out-dated, see comments below) Semantic change to `SetDestroyCallback()` ~~Currently, the method overwrites the old callback with the new one. With this PR, it will NOT overwrite. Instead, it will hold on to both. Both callbacks get called during destroy.~~ ~~**Risk**: Although the documentation of `SetDestroyCallback()` (see [C++](https://lldb.llvm.org/cpp_reference/classlldb_1_1SBDebugger.html#afa1649d9453a376b5c95888b5a0cb4ec) and [python](https://lldb.llvm.org/python_api/lldb.SBDebugger.html#lldb.SBDebugger.SetDestroyCallback)) doesn't really specify the behavior, there is a risk: if existing call sites rely on the "overwrite" behavior, they will be surprised because now the old callback will get called. But as the above said, the current behavior of "overwrite" itself might be unintended, so I don't anticipate users to rely on this behavior. In short, this risk might be less of a problem if we correct it sooner rather than later (which is what this PR is trying to do).~~ ## (out-dated, see comments below) Implementation ~~The implementation holds a `std::vector<std::pair<callback, baton>>`. When `SetDestroyCallback()` is called, callbacks and batons are appended to the `std::vector`. When destroy event happen, the `(callback, baton)` pairs are invoked FIFO. Finally, the `std::vector` is cleared.~~ # (out-dated, see comments below) Alternatives considered ~~Instead of changing `SetDestroyCallback()`, a new method `AddDestroyCallback()` can be added, which use the same `std::vector<std::pair<>>` implementation. Together with `ClearDestroyCallback()` (see below), they will replace and deprecate `SetDestroyCallback()`. Meanwhile, in order to be backward compatible, `SetDestroyCallback()` need to be updated to clear the `std::vector` and then add the new callback. Pros: The end state is semantically more correct. Cons: More steps to take; potentially maintaining an "incorrect" behavior (of "overwrite").~~ ~~A new method `ClearDestroyCallback()` can be added. Might be unnecessary at this point, because workflows which need to set then clear callbacks may exist but shouldn't be too common at least for now. Such method can be added later when needed.~~ ~~The `std::vector` may bring slight performance drawback if its implementation doesn't handle small size efficiently. However, even if that's the case, this path should be very cold (only used during init and destroy). Such performance drawback should be negligible.~~ ~~A different implementation was also considered. Instead of using `std::vector`, the current `m_destroy_callback` field can be kept unchanged. When `SetDestroyCallback()` is called, a lambda function can be stored into `m_destroy_callback`. This lambda function will first call the old callback, then the new one. This way, `std::vector` is avoided. However, this implementation is more complex, thus less readable, with not much perf to gain.~~ --------- Co-authored-by: Roy Shi <royshi@meta.com>
Diffstat (limited to 'lldb/source/API/SBDebugger.cpp')
-rw-r--r--lldb/source/API/SBDebugger.cpp20
1 files changed, 20 insertions, 0 deletions
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 9c662df..7ef0d6e 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1695,6 +1695,26 @@ void SBDebugger::SetDestroyCallback(
}
}
+lldb::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);
+
+ return LLDB_INVALID_CALLBACK_TOKEN;
+}
+
+bool SBDebugger::RemoveDestroyCallback(lldb::callback_token_t token) {
+ LLDB_INSTRUMENT_VA(this, token);
+
+ if (m_opaque_sp)
+ return m_opaque_sp->RemoveDestroyCallback(token);
+
+ return false;
+}
+
SBTrace
SBDebugger::LoadTraceFromFile(SBError &error,
const SBFileSpec &trace_description_file) {