From 93efe4ef963ba3b0b88bc7e7ca86484f0300a094 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Sat, 24 Feb 2024 14:53:21 +0100 Subject: [PATCH 1/7] fix: ensure proper order of signal emmision --- src/psygnal/_signal.py | 33 +++++++++++++++++++++++++-------- tests/test_psygnal.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 8 deletions(-) diff --git a/src/psygnal/_signal.py b/src/psygnal/_signal.py index 22ce805e..e46b5612 100644 --- a/src/psygnal/_signal.py +++ b/src/psygnal/_signal.py @@ -299,6 +299,9 @@ class Emitter: Whether to check the callback parameter types against `signature` when connecting a new callback. This can also be provided at connection time using `.connect(..., check_types=True)`. By default, False. + raise_on_emit_during_emission: bool + raise exception if signal is emitted while already emitting. + This could be used to detect callback loops. By default, False. Raises ------ @@ -319,6 +322,7 @@ def __init__( name: str | None = None, check_nargs_on_connect: bool = True, check_types_on_connect: bool = False, + raise_on_emit_during_emission: bool = False, ) -> None: self._name = name self._instance: Callable = self._instance_ref(instance) @@ -335,10 +339,12 @@ def __init__( self._signature = signature self._check_nargs_on_connect = check_nargs_on_connect self._check_types_on_connect = check_types_on_connect + self._raise_on_emit_during_emission = raise_on_emit_during_emission self._slots: list[WeakCallback] = [] self._is_blocked: bool = False self._is_paused: bool = False self._lock = threading.RLock() + self._emit_queue: list[tuple] = [] @staticmethod def _instance_ref(instance: Any) -> Callable[[], Any]: @@ -1034,14 +1040,25 @@ def __call__( def _run_emit_loop(self, args: tuple[Any, ...]) -> None: # allow receiver to query sender with Signal.current_emitter() with self._lock: - with Signal._emitting(self): - for caller in self._slots: - try: - caller.cb(args) - except Exception as e: - raise EmitLoopError( - cb=caller, args=args, exc=e, signal=self - ) from e + self._emit_queue.append(args) + if len(self._emit_queue) > 1: + if self._raise_on_emit_during_emission: + raise RuntimeError( + f"Signal {self!r} emitted while already emitting." + ) + return None + try: + with Signal._emitting(self): + i = 0 + while i < len(self._emit_queue): + arg = self._emit_queue[i] + for caller in self._slots: + caller.cb(arg) + i += 1 + except Exception as e: + raise EmitLoopError(cb=caller, args=args, exc=e, signal=self) from e + finally: + self._emit_queue.clear() return None diff --git a/tests/test_psygnal.py b/tests/test_psygnal.py index b5f11990..1e5ad300 100644 --- a/tests/test_psygnal.py +++ b/tests/test_psygnal.py @@ -996,3 +996,39 @@ def test_pickle(): x = pickle.loads(_dump) x.sig.emit() mock.assert_called_once() + + +def test_signal_order(): + """Test that signals are emitted in the order they were connected.""" + emitter = Emitter() + mock1 = Mock() + mock2 = Mock() + + def callback(x): + if x != 10: + emitter.one_int.emit(10) + + emitter.one_int.connect(mock1) + emitter.one_int.connect(callback) + emitter.one_int.connect(mock2) + emitter.one_int.emit(1) + + mock1.assert_has_calls([call(1), call(10)]) + mock2.assert_has_calls([call(1), call(10)]) + + +def test_double_emmision_error(): + s = SignalInstance(raise_on_emit_during_emission=True) + + i = 0 + + def callback(): + nonlocal i + if i == 0: + s.emit() + i += 1 + + s.connect(callback) + + with pytest.raises(EmitLoopError): + s.emit() From 3e8be849df45c192e0ef043f1e4053e7b291463f Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Sat, 24 Feb 2024 15:12:22 +0100 Subject: [PATCH 2/7] fix: copy operator --- src/psygnal/_signal.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/psygnal/_signal.py b/src/psygnal/_signal.py index e46b5612..d188f895 100644 --- a/src/psygnal/_signal.py +++ b/src/psygnal/_signal.py @@ -1207,6 +1207,7 @@ def __getstate__(self) -> dict: "_args_queue", "_check_nargs_on_connect", "_check_types_on_connect", + "_emit_queue", ) dd = {slot: getattr(self, slot) for slot in attrs} dd["_instance"] = self._instance() From af5ebfe302612acd302244d2e1fa06aa8a900a28 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Mon, 26 Feb 2024 14:02:17 +0100 Subject: [PATCH 3/7] remove exception raise --- src/psygnal/_signal.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/psygnal/_signal.py b/src/psygnal/_signal.py index d188f895..d6c99c64 100644 --- a/src/psygnal/_signal.py +++ b/src/psygnal/_signal.py @@ -299,9 +299,6 @@ class Emitter: Whether to check the callback parameter types against `signature` when connecting a new callback. This can also be provided at connection time using `.connect(..., check_types=True)`. By default, False. - raise_on_emit_during_emission: bool - raise exception if signal is emitted while already emitting. - This could be used to detect callback loops. By default, False. Raises ------ @@ -322,7 +319,6 @@ def __init__( name: str | None = None, check_nargs_on_connect: bool = True, check_types_on_connect: bool = False, - raise_on_emit_during_emission: bool = False, ) -> None: self._name = name self._instance: Callable = self._instance_ref(instance) @@ -339,7 +335,6 @@ def __init__( self._signature = signature self._check_nargs_on_connect = check_nargs_on_connect self._check_types_on_connect = check_types_on_connect - self._raise_on_emit_during_emission = raise_on_emit_during_emission self._slots: list[WeakCallback] = [] self._is_blocked: bool = False self._is_paused: bool = False @@ -1042,10 +1037,6 @@ def _run_emit_loop(self, args: tuple[Any, ...]) -> None: with self._lock: self._emit_queue.append(args) if len(self._emit_queue) > 1: - if self._raise_on_emit_during_emission: - raise RuntimeError( - f"Signal {self!r} emitted while already emitting." - ) return None try: with Signal._emitting(self): From accd0b33727af13bf556b9e2fc6c95496cbbc1a4 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Mon, 26 Feb 2024 14:06:41 +0100 Subject: [PATCH 4/7] fix tests --- tests/test_psygnal.py | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/tests/test_psygnal.py b/tests/test_psygnal.py index 1e5ad300..1dcc4dea 100644 --- a/tests/test_psygnal.py +++ b/tests/test_psygnal.py @@ -1017,18 +1017,32 @@ def callback(x): mock2.assert_has_calls([call(1), call(10)]) -def test_double_emmision_error(): - s = SignalInstance(raise_on_emit_during_emission=True) +def test_signal_order_suspend(): + """Test that signals are emitted in the order they were connected.""" + emitter = Emitter() + mock1 = Mock() + mock2 = Mock() + + def callback(x): + if x < 10: + emitter.one_int.emit(10) - i = 0 + def callback2(x): + if x == 10: + emitter.one_int.emit(11) - def callback(): - nonlocal i - if i == 0: - s.emit() - i += 1 + def callback3(x): + if x == 10: + with emitter.one_int.paused(reducer=lambda a, b: (a[0] + b[0],)): + for i in range(12, 15): + emitter.one_int.emit(i) - s.connect(callback) + emitter.one_int.connect(mock1) + emitter.one_int.connect(callback) + emitter.one_int.connect(callback2) + emitter.one_int.connect(callback3) + emitter.one_int.connect(mock2) + emitter.one_int.emit(1) - with pytest.raises(EmitLoopError): - s.emit() + mock1.assert_has_calls([call(1), call(10), call(11), call(39)]) + mock2.assert_has_calls([call(1), call(10), call(11), call(39)]) From 4741d455750a79e4cf2540bb22204102ffc26316 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Mon, 26 Feb 2024 16:58:37 +0100 Subject: [PATCH 5/7] make recursion real again --- src/psygnal/_signal.py | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/psygnal/_signal.py b/src/psygnal/_signal.py index dbfd2036..5a811cd8 100644 --- a/src/psygnal/_signal.py +++ b/src/psygnal/_signal.py @@ -945,6 +945,25 @@ def __call__( check_types=check_types, ) + def _run_emit_loop_inner(self) -> None: + try: + with Signal._emitting(self): + q_len = len(self._emit_queue) + for i in range(q_len): + args = self._emit_queue[i] + for caller in self._slots: + caller.cb(args) + self._emit_queue = self._emit_queue[q_len:] + if len(self._emit_queue) > 0: + self._run_emit_loop_inner() + except RecursionError as e: + raise RecursionError( + f"RecursionError in {caller.slot_repr()} when " + f"emitting signal {self.name!r} with args {args}" + ) from e + except Exception as e: + raise EmitLoopError(cb=caller, args=args, exc=e, signal=self) from e + def _run_emit_loop(self, args: tuple[Any, ...]) -> None: # allow receiver to query sender with Signal.current_emitter() with self._lock: @@ -952,20 +971,7 @@ def _run_emit_loop(self, args: tuple[Any, ...]) -> None: if len(self._emit_queue) > 1: return None try: - with Signal._emitting(self): - i = 0 - while i < len(self._emit_queue): - arg = self._emit_queue[i] - for caller in self._slots: - caller.cb(arg) - i += 1 - except RecursionError as e: - raise RecursionError( - f"RecursionError in {caller.slot_repr()} when " - f"emitting signal {self.name!r} with args {args}" - ) from e - except Exception as e: - raise EmitLoopError(cb=caller, args=args, exc=e, signal=self) from e + self._run_emit_loop_inner() finally: self._emit_queue.clear() From f62dfd6bf54e5a3e386e93cbf9b17a6e9489386a Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Mon, 26 Feb 2024 17:02:00 +0100 Subject: [PATCH 6/7] ass test for evented set --- tests/containers/test_evented_set.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/containers/test_evented_set.py b/tests/containers/test_evented_set.py index cfa4cbcc..16a0ddf9 100644 --- a/tests/containers/test_evented_set.py +++ b/tests/containers/test_evented_set.py @@ -141,3 +141,29 @@ def test_copy_no_sync(): s2 = copy(s1) s1.add(4) assert len(s2) == 3 + + +def test_set_emission_order(): + s = EventedSet() + + def callback1(): + if 1 not in s: + s.add(1) + + def callback2(): + if 5 not in s: + s.update(range(5, 10)) + + s.events.items_changed.connect(callback1) + s.events.items_changed.connect(callback2) + mock = Mock() + s.events.items_changed.connect(mock) + + s.add(11) + mock.assert_has_calls( + [ + call((11,), ()), + call((1,), ()), + call((5, 6, 7, 8, 9), ()), + ] + ) From eccef1a5136310bc2e93085b93da64caf17d28b3 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Mon, 26 Feb 2024 23:58:59 +0100 Subject: [PATCH 7/7] simplify code --- src/psygnal/_signal.py | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/src/psygnal/_signal.py b/src/psygnal/_signal.py index 5a811cd8..60f56845 100644 --- a/src/psygnal/_signal.py +++ b/src/psygnal/_signal.py @@ -1,6 +1,7 @@ from __future__ import annotations import inspect +import sys import threading import warnings import weakref @@ -52,6 +53,7 @@ __all__ = ["Signal", "SignalInstance", "_compiled"] _NULL = object() F = TypeVar("F", bound=Callable) +RECURSION_LIMIT = sys.getrecursionlimit() class Signal: @@ -945,33 +947,30 @@ def __call__( check_types=check_types, ) - def _run_emit_loop_inner(self) -> None: - try: - with Signal._emitting(self): - q_len = len(self._emit_queue) - for i in range(q_len): - args = self._emit_queue[i] - for caller in self._slots: - caller.cb(args) - self._emit_queue = self._emit_queue[q_len:] - if len(self._emit_queue) > 0: - self._run_emit_loop_inner() - except RecursionError as e: - raise RecursionError( - f"RecursionError in {caller.slot_repr()} when " - f"emitting signal {self.name!r} with args {args}" - ) from e - except Exception as e: - raise EmitLoopError(cb=caller, args=args, exc=e, signal=self) from e - def _run_emit_loop(self, args: tuple[Any, ...]) -> None: # allow receiver to query sender with Signal.current_emitter() with self._lock: self._emit_queue.append(args) + if len(self._emit_queue) > 1: return None try: - self._run_emit_loop_inner() + with Signal._emitting(self): + i = 0 + while i < len(self._emit_queue): + args = self._emit_queue[i] + for caller in self._slots: + caller.cb(args) + if len(self._emit_queue) > RECURSION_LIMIT: + raise RecursionError + i += 1 + except RecursionError as e: + raise RecursionError( + f"RecursionError in {caller.slot_repr()} when " + f"emitting signal {self.name!r} with args {args}" + ) from e + except Exception as e: + raise EmitLoopError(cb=caller, args=args, exc=e, signal=self) from e finally: self._emit_queue.clear()