Merge branch 'bf/notifier' into 'main'

BF: Fix to `Notifier` class in its handling of garbage-collected callback functions

See merge request fsl/fslpy!470

Author: Paul McCarthy

CHANGELOG.rst

@@ -10,6 +10,8 @@ Fixed
 
 * Adjusted the :func:`.featdesign.loadFEATDesignFile` function to handle missing
   values (!469).
+* Fix to the :class:`.Notifier` class involving handling of callback functions
+  that have been garbage-collected (!470).
 
 
 3.22.0 (Monday 17th February 2025)

fsl/tests/test_notifier.py

@@ -204,3 +204,31 @@ def topic_callback(*a):
     t.notify(topic='topic')
     assert default_called[0] == 14
     assert topic_called[  0] == 6
+
+
+# Make sure there is no error
+# if a callback function is GC'd
+# fsl/fslpy!470
+def test_gc():
+
+    class Thing(notifier.Notifier):
+        pass
+
+    t = Thing()
+
+    called = []
+
+    def callback(thing, topic, value):
+        called.append((thing, topic, value))
+
+    t.register('callback', callback)
+
+    t.notify()
+    assert called == [(t, None, None)]
+
+    called[:] = []
+    callback  = None
+    del callback
+
+    t.notify()
+    assert called == []

fsl/utils/notifier.py

@@ -66,7 +66,12 @@ def expectsArguments(self):
         positional arguments - see :meth:`Notifier.register` for details.
         """
 
-        func    = self.callback
+        func = self.callback
+
+        # the function may have been GC'd
+        if func is None:
+            return False
+
         spec    = inspect.signature(func)
         posargs = 0
         varargs = False
@@ -377,22 +382,23 @@ def notify(self, *args, **kwargs):
             callback = listener.callback
             name     = listener.name
 
-            if listener.expectsArguments: args = (self, topic, value)
-            else:                         args = ()
-
             # The callback, or the owner of the
             # callback function may have been
             # gc'd - remove it if this is the case.
             if callback is None:
                 log.debug('Listener %s has been gc\'d - '
                           'removing from list', name)
                 self.__listeners[listener.topic].pop(name)
+                continue
 
-            elif not listener.enabled:
+            if not listener.enabled:
                 continue
 
-            elif listener.runOnIdle: idle.idle(callback, *args)
-            else:                    callback(           *args)
+            if listener.expectsArguments: args = (self, topic, value)
+            else:                         args = ()
+
+            if listener.runOnIdle: idle.idle(callback, *args)
+            else:                  callback(           *args)
 
 
     def __getListeners(self, topic):