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>

1 files changed, 121 insertions, 0 deletions

diff --git a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
index 522de2466012..29b8cfadd947 100644
--- a/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
+++ b/lldb/test/API/python_api/debugger/TestDebuggerAPI.py
@@ -161,3 +161,124 @@ class DebuggerAPITestCase(TestBase):
         original_dbg_id = self.dbg.GetID()
         self.dbg.Destroy(self.dbg)
         self.assertEqual(destroy_dbg_id, original_dbg_id)
+
+    def test_AddDestroyCallback(self):
+        original_dbg_id = self.dbg.GetID()
+        called = []
+
+        def foo(dbg_id):
+            # Need nonlocal to modify closure variable.
+            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)
+        self.dbg.Destroy(self.dbg)
+
+        # Should call both `foo()` and `bar()`.
+        self.assertEqual(called, [
+            ('foo', original_dbg_id),
+            ('bar', original_dbg_id),
+        ])
+
+    def test_RemoveDestroyCallback(self):
+        original_dbg_id = self.dbg.GetID()
+        called = []
+
+        def foo(dbg_id):
+            # Need nonlocal to modify closure variable.
+            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)]
+
+        token_foo = self.dbg.AddDestroyCallback(foo)
+        ret = self.dbg.RemoveDestroyCallback(magic_token_that_should_not_exist)
+        self.dbg.Destroy(self.dbg)
+
+        # `Remove` should be unsuccessful
+        self.assertFalse(ret)
+        # Should call `foo()`
+        self.assertEqual(called, [('foo', original_dbg_id)])
+
+    def test_HandleDestroyCallback(self):
+        """
+        Validates:
+        1. AddDestroyCallback and RemoveDestroyCallback work during debugger destroy.
+        2. HandleDestroyCallback invokes all callbacks in FIFO order.
+        """
+        original_dbg_id = self.dbg.GetID()
+        events = []
+        bar_token = None
+
+        def foo(dbg_id):
+            # Need nonlocal to modify closure variable.
+            nonlocal events
+            events.append(('foo called', dbg_id))
+
+        def bar(dbg_id):
+            # Need nonlocal to modify closure variable.
+            nonlocal events
+            events.append(('bar called', dbg_id))
+
+        def add_foo(dbg_id):
+            # Need nonlocal to modify closure variable.
+            nonlocal events
+            events.append(('add_foo called', dbg_id))
+            events.append(('foo token', self.dbg.AddDestroyCallback(foo)))
+
+        def remove_bar(dbg_id):
+            # Need nonlocal to modify closure variable.
+            nonlocal events
+            events.append(('remove_bar called', dbg_id))
+            events.append(('remove bar ret', self.dbg.RemoveDestroyCallback(bar_token)))
+
+        # Setup
+        events.append(('add_foo token', self.dbg.AddDestroyCallback(add_foo)))
+        bar_token = self.dbg.AddDestroyCallback(bar)
+        events.append(('bar token', bar_token))
+        events.append(('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 called', original_dbg_id), # bar should be called
+            ('remove_bar called', original_dbg_id), # remove_bar should be called
+            ('remove bar ret', False), # remove_bar should fail, because it's already invoked and removed
+            ('foo called', original_dbg_id), # foo should be called
+        ])