From 90df38b372670a16e667b2bb448e1b0afd095272 Mon Sep 17 00:00:00 2001 From: Daniel Werner Date: Thu, 31 Aug 2023 10:46:10 +0200 Subject: [PATCH 1/5] Fix dead objects being called through weakrefs When events (such as 'tensorlib_changed') are called, a bug occured previously where objects (i.e. the `TensorViewer`) are already dead. In this case an error occured when member functions where using attributes such as `self._sorted_indices`. This bug is fixed by catching the cases, in which the object is invalidated by earlier callbacks. --- src/pyhf/events.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/pyhf/events.py b/src/pyhf/events.py index f28d0a7563..d61e91a8ad 100644 --- a/src/pyhf/events.py +++ b/src/pyhf/events.py @@ -64,12 +64,19 @@ 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) + # We have to flush after calling all the callbacks, not before. That's + # beacause, earlier callbacks might cause new dead arg weakrefs in + # later callbacks. So we check for dead weakrefs in each iteration + # and then we flush at the end. + self._flush() def __iter__(self): return iter(self.callbacks) From b1388979b6541e2a0162e26351d7ce3dd0e934d5 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 31 Aug 2023 09:35:44 +0000 Subject: [PATCH 2/5] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/pyhf/events.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pyhf/events.py b/src/pyhf/events.py index d61e91a8ad..a57126b924 100644 --- a/src/pyhf/events.py +++ b/src/pyhf/events.py @@ -73,7 +73,7 @@ def __call__(self, *args, **kwargs): else: func()(*args, **kwargs) # We have to flush after calling all the callbacks, not before. That's - # beacause, earlier callbacks might cause new dead arg weakrefs in + # because, earlier callbacks might cause new dead arg weakrefs in # later callbacks. So we check for dead weakrefs in each iteration # and then we flush at the end. self._flush() From 6f95bcab238d215b849eb3bf6fb1080cdb2d9bca Mon Sep 17 00:00:00 2001 From: Matthew Feickert Date: Tue, 5 Sep 2023 12:01:49 -0500 Subject: [PATCH 3/5] shorten summary --- src/pyhf/events.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/pyhf/events.py b/src/pyhf/events.py index a57126b924..e1864e6d93 100644 --- a/src/pyhf/events.py +++ b/src/pyhf/events.py @@ -72,10 +72,9 @@ def __call__(self, *args, **kwargs): func()(arg_ref, *args, **kwargs) else: func()(*args, **kwargs) - # We have to flush after calling all the callbacks, not before. That's - # because, earlier callbacks might cause new dead arg weakrefs in - # later callbacks. So we check for dead weakrefs in each iteration - # and then we flush at the end. + # flush after calling all the callbacks, not before, as earlier + # callbacks might cause new dead arg weakrefs in later callbacks. + # So check for dead weakrefs in each iteration and flush at the end. self._flush() def __iter__(self): From bc288e979a0887f94f6ac3641a8a8835cf56c88a Mon Sep 17 00:00:00 2001 From: Matthew Feickert Date: Wed, 6 Sep 2023 14:30:33 -0500 Subject: [PATCH 4/5] Add more detailed comment Co-authored-by: Jonas Rembser --- src/pyhf/events.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/pyhf/events.py b/src/pyhf/events.py index e1864e6d93..ae55e2d4f1 100644 --- a/src/pyhf/events.py +++ b/src/pyhf/events.py @@ -72,9 +72,11 @@ def __call__(self, *args, **kwargs): func()(arg_ref, *args, **kwargs) else: func()(*args, **kwargs) - # flush after calling all the callbacks, not before, as earlier - # callbacks might cause new dead arg weakrefs in later callbacks. - # So check for dead weakrefs in each iteration and flush at the end. + # 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): From 5cb11ae0918765b839ada4b23fe8ac9524436ed8 Mon Sep 17 00:00:00 2001 From: Matthew Feickert Date: Wed, 6 Sep 2023 14:35:30 -0500 Subject: [PATCH 5/5] Add Jonas to contributors given the level of detailed explentation in debugging --- docs/contributors.rst | 1 + 1 file changed, 1 insertion(+) 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