[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)

[via lldb-commits]

================
@@ -743,9 +743,19 @@ 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;
+  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.
+  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);
+    }
----------------
royitaqi wrote:

Updated the PR accordingly.

--

(Regardless of the loop implementation.) However, it came to me that the behavior of removing a callback from an existing callback can be **inconsistant**: regardless of the order of registration, if the remov**er** is invoked first, the remov**ee** is removed and never invoked; meanwhile, if the remov**ee** is invoked first, it is invoked and instead the removal returns `false`.

I am wondering if the container should be changed to `std::map`, so that the order and behavior will be consistent. I.e. earlier-registered callbacks always gets invoked before later-registered ones. This also means that the loop can simply be to iterate through and erase until `.end()` - because added callbacks will always be inserted after the current one.

Perf-wise this shouldn't make a difference, since this is on the destroy path, and assuming there isn't too many callbacks added.

**LMK what you think.**

https://github.com/llvm/llvm-project/pull/89868