Callback.disconnect_all() appears to take a long winded approach to result in self.__callback_map.clear(). What advantage does the current implementation offer?
Thanks @dsblank. I had looked and seen that commit from ~15 years ago. That commit introduced the method in its current form. So, I gave claude the code, some context and my understanding of the type hint for __callback_map (Note: instead of Callable it should be types.FunctionType | types.MethodType). Claude gave the following analysis.
Analysis of disconnect_all vs self.__callback_map.clear()
Data structure
__callback_map has the type dict[str, list[tuple[int, Callable]]]. The callables are typically bound methods of GUI widgets, meaning each tuple holds a strong reference to the widget it belongs to. External code only ever holds the integer callback id — nothing outside the class can reference the inner lists or tuples directly.
The “lapsed listener” risk
The primary memory concern here is not structural but lifecycle-based. A bound method like widget.on_update holds a strong reference to widget. Until a tuple containing that callable is freed, the widget cannot be garbage collected even if every other reference to it is gone. This is a real risk in Gramps, where many GUI components connect to database signals and are expected to be cleanly destroyed when views are closed.
Does .clear() handle this correctly?
Yes — provided __callback_map is the sole owner of those tuples, which it is given the type contract. CPython’s reference counting will immediately cascade: the inner lists are freed, then the tuples, then the callables, then the widgets’ refcounts are decremented. No leak.
What the verbose version actually guards against
The one scenario where the verbose version’s in-place remove() loop could matter is if emit() is iterating over a signal’s inner list at the moment disconnect_all() is called:
python
# inside emit()
for id, cb in self.__callback_map[signal_name]: # iterator holds ref to list
cb(*args)
During this loop, the iterator itself holds a reference to the inner list. Calling .clear() on the outer dict drops the dict’s reference, but the iterator keeps the list — and therefore all its callables and their widgets — alive until the loop finishes. Mutating the list in-place via remove() would affect the same object the iterator is traversing.
However, the copy.copy() in the verbose version defeats this entirely:
python
keymap = copy.copy(self.__callback_map[signal_name]) # snapshot
for key in keymap: # iterates snapshot
self.__callback_map[signal_name].remove(key) # mutates original
It iterates a copy but mutates the original — the opposite of what you’d want to affect an in-progress emit() iterator. The in-place mutation benefit is completely negated.
Additionally, the remove() loop is O(n²) per signal — each remove() does a linear scan of the list — for no gain.
Recommendation
Replace with .clear() — but with one caveat.
If emit() iterates directly over the live inner list (as shown above), then both the verbose version and .clear() leave a transient window where freed widgets are kept alive for the duration of an in-progress emit. This should be fixed in emit() by iterating over a copy:
python
for id, cb in list(self.__callback_map.get(signal_name, [])):
cb(*args)
With that fix in place, disconnect_all becomes correctly and completely:
python
def disconnect_all(self) -> None:
"""Disconnect all callbacks."""
self.__callback_map.clear()
The original verbose version is O(n²), contains a logical contradiction (copies the list to then mutate it in-place), and provides no memory safety benefit over .clear() given the established type contract. The underlying widget lifetime concern it was likely written to address is real, but is not solved by the cleanup strategy — it should be addressed either in emit() or, more robustly, by storing weak references to callables in connect().
Looks good! Thanks for digging into this.