From 1d43c7ef7e586b0ddd9f74870241fa3ab1288ee2 Mon Sep 17 00:00:00 2001 From: Daniel Werner <138122408+wernerd-cern@users.noreply.github.com> Date: Wed, 6 Sep 2023 21:54:39 +0200 Subject: [PATCH] fix: Skip callbacks with dead weakrefs while iterating over callbacks (#2310) * Check refs while processing callbacks in case a callee is de-ref'd during the callback process. * Additionally flush after processing callbacks in case any callback de-ref's a callee. * Note that no additional tests are added for this case as the problem arises from and intermediate callback triggering a garbage-collect. It is unclear how to force this scenario deterministically in testing. * Add Jonas Rembser to contributors list. Co-authored-by: Jonas Rembser --- docs/contributors.rst | 1 + src/pyhf/events.py | 12 ++++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/docs/contributors.rst b/docs/contributors.rst index 08a828bf19..c2d0b0a698 100644 --- a/docs/contributors.rst +++ b/docs/contributors.rst @@ -32,3 +32,4 @@ Contributors include: - Nathan Simpson - Beojan Stanislaus - Daniel Werner +- Jonas Rembser diff --git a/src/pyhf/events.py b/src/pyhf/events.py index f28d0a7563..ae55e2d4f1 100644 --- a/src/pyhf/events.py +++ b/src/pyhf/events.py @@ -64,12 +64,20 @@ def _flush(self): self._callbacks = _callbacks def __call__(self, *args, **kwargs): - for func, arg in self.callbacks: + for func, arg in self._callbacks: # weakref: needs to be de-ref'd first before calling if arg is not None: - func()(arg(), *args, **kwargs) + arg_ref = arg() + if arg_ref is not None: + func()(arg_ref, *args, **kwargs) else: func()(*args, **kwargs) + # Flush after calling all the callbacks, not before, as callbacks in the + # beginning of the iteration might cause new dead arg weakrefs in + # callbacks that are iterated over later. + # Checking for dead weakrefs in each iteration and flushing at the end + # avoids redundant dead weakref checking in subsequent calls. + self._flush() def __iter__(self): return iter(self.callbacks)