Skip to content

Commit

Permalink
fix: Skip callbacks with dead weakrefs while iterating over callbacks (
Browse files Browse the repository at this point in the history
…#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 <jonas.rembser@cern.ch>
  • Loading branch information
wernerd-cern and guitargeek authored Sep 6, 2023
1 parent 4954010 commit 1d43c7e
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 2 deletions.
1 change: 1 addition & 0 deletions docs/contributors.rst
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,4 @@ Contributors include:
- Nathan Simpson
- Beojan Stanislaus
- Daniel Werner
- Jonas Rembser
12 changes: 10 additions & 2 deletions src/pyhf/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 1d43c7e

Please sign in to comment.