From 746cf071ca28b9bcd2ac8a3b716208073b28aba7 Mon Sep 17 00:00:00 2001 From: "Eloise Y." <98988862+eloise-nebula@users.noreply.github.com> Date: Wed, 17 Sep 2025 16:50:29 -0700 Subject: [PATCH 1/4] ActionLoop thread now handles _destroy. Reading __destroyed is now threadsafe. --- pykern/pkasyncio.py | 51 +++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/pykern/pkasyncio.py b/pykern/pkasyncio.py index 07b67061..ef3f27ca 100644 --- a/pykern/pkasyncio.py +++ b/pykern/pkasyncio.py @@ -49,7 +49,7 @@ class ActionLoop: def __init__(self): # All these attributes must exist even after destroy() - self.destroyed = False + self.__destroyed = False self.__lock = threading.Lock() self.__actions = queue.Queue() # You can join the thread to block another (e.g. main) thread from exiting @@ -68,9 +68,7 @@ def action(self, method, arg): """Queue ``method`` to be called in loop thread. Actions are methods that (by convention) begin with - ``action_`` and are called sequentially inside `_start`. A - lock is used to prevent `destroy` being called during the - action and serializing activities within a single action. + ``action_`` and are called sequentially inside `_start`. Actions return ``None`` to continue on to the next action. `_LOOP_END` should be returned to terminate `_start` @@ -106,21 +104,18 @@ def action(self, method, arg): ) def destroy(self): - """Stops thread and calls subclass `_destroy` + """Stops thread. - THREADING: subclasses should not call destroy directly. They should - return `_LOOP_END` instead. External callbacks may call destroy, because - _ActionLoop does not hold lock during external callbacks. + THREADING: subclasses and external callbacks may call destroy directly. """ - try: - with self.__lock: - if self.destroyed: - return - self.destroyed = True - self.__actions.put_nowait((None, None)) - self._destroy() - except Exception as e: - pkdlog("error={} {} stack={}", e, self, pkdexc(simplify=True)) + with self.__lock: + self.__destroyed = True + + def is_destroyed(self): + """Thread-safe check if ActionLoop is destroyed. + """ + with self.__lock: + return self.__destroyed def _dispatch_action(self, method, arg): """Calls method with arg. @@ -161,7 +156,7 @@ def _on_loop_timeout(self): def __repr__(self): def _destroyed(): - return " DESTROYED" if self.destroyed else "" + return " DESTROYED" if self.is_destroyed else "" return f"<{self.__class__.__name__}{_destroyed()} self._repr()>" @@ -174,7 +169,7 @@ def _start(self): """ while True: with self.__lock: - if self.destroyed: + if self.__destroyed: return try: m, a = self.__actions.get(**self.__get_args) @@ -182,17 +177,23 @@ def _start(self): except queue.Empty: m, a = self._on_loop_timeout(), None with self.__lock: - if self.destroyed: - return + if self.__destroyed: + break # Do not need to check m, because only invalid when destroyed is True - if (m := self._dispatch_action(m, a)) is self._LOOP_END: - return + if (m := self._dispatch_action(m, a)) is self._LOOP_END: + break # Will be true if destroy called inside action (m) - if self.destroyed: - return + with self.__lock: + if self.__destroyed: + break # Action returned an external callback, which must occur outside lock if m: self._dispatch_callback(m) + with self.__lock: + # Can this happen? + if not self.__destroyed: + return + self._destroy() def __target(self): """Thread's target function""" From f626ee533577b178bb418ac731e9944219406009 Mon Sep 17 00:00:00 2001 From: "Eloise Y." <98988862+eloise-nebula@users.noreply.github.com> Date: Tue, 23 Sep 2025 15:48:09 -0700 Subject: [PATCH 2/4] chkp --- pykern/pkasyncio.py | 76 ++++++++++++++++++++++++--------------------- 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/pykern/pkasyncio.py b/pykern/pkasyncio.py index ef3f27ca..d622514a 100644 --- a/pykern/pkasyncio.py +++ b/pykern/pkasyncio.py @@ -104,18 +104,19 @@ def action(self, method, arg): ) def destroy(self): - """Stops thread. + """Stops the thread. + + `_start` always calls the `_handle_destroy` callback at the end of the + loop. + + Subclasses should not reimplement. To handle behavior on destroy, + call `_handle_destroy`. THREADING: subclasses and external callbacks may call destroy directly. """ with self.__lock: self.__destroyed = True - - def is_destroyed(self): - """Thread-safe check if ActionLoop is destroyed. - """ - with self.__lock: - return self.__destroyed + self.__actions.put_nowait((None, None)) def _dispatch_action(self, method, arg): """Calls method with arg. @@ -123,8 +124,6 @@ def _dispatch_action(self, method, arg): Subclasses may re-implement. This function will remain a very simple wrapper for ``return method(arg)``. - This function is called inside the lock. - Args: method (callable): to be called arg (object): to be passed @@ -139,13 +138,33 @@ def _dispatch_callback(self, callback): Subclasses may re-implement. This method will remain a very simple wrapper for ``callback()``. - This function is called outside the lock. - Args: callback (callable): to be called """ callback() + def _handle_destroy(self): + """Callback on destroy and loop end. + + Subclasses may re-implement. + + THREADING: subclasses should not call directly. This callback is + handled by the thread. Instead, call `destroy`. + """ + pass + + def _handle_exception(self, exc): + """Exception handler for `_start`. + + `_handle_exception` is called when there's an exception in `_start`. + + Subclasses may reimplement. + + Args: + exc (Exception): Captured Exception. + """ + pass + def _on_loop_timeout(self): """Called when a loop timeout occurs. @@ -154,12 +173,6 @@ def _on_loop_timeout(self): # `__init__` prevents this from happening, but good to document. raise NotImplementedError("ActionLoop._on_loop_timeout") - def __repr__(self): - def _destroyed(): - return " DESTROYED" if self.is_destroyed else "" - - return f"<{self.__class__.__name__}{_destroyed()} self._repr()>" - def _start(self): """Loops over actions and exits on `_LOOP_END` or on unhandled exception. @@ -170,7 +183,7 @@ def _start(self): while True: with self.__lock: if self.__destroyed: - return + break try: m, a = self.__actions.get(**self.__get_args) self.__actions.task_done() @@ -186,14 +199,17 @@ def _start(self): with self.__lock: if self.__destroyed: break - # Action returned an external callback, which must occur outside lock + # Action returned an external callback if m: self._dispatch_callback(m) - with self.__lock: - # Can this happen? - if not self.__destroyed: - return - self._destroy() + # Callback on destroy and loop end. + self._handle_destroy() + + def __repr__(self): + def _destroyed(): + return " DESTROYED" if self.__destroyed else "" + + return f"<{self.__class__.__name__}{_destroyed()} self._repr()>" def __target(self): """Thread's target function""" @@ -213,18 +229,6 @@ def __target(self): finally: self.destroy() - def _handle_exception(self, exc): - """Exception handler for `_start`. - - `_handle_exception` is called when there's an exception in `_start`. - - Subclasses may reimplement. - - Args: - exc (Exception): Captured Exception. - """ - pass - class Loop: """HTTP Server loop""" From 698e474d92c071c9c559e6ccaaafdf25163b90e0 Mon Sep 17 00:00:00 2001 From: "Eloise Y." <98988862+eloise-nebula@users.noreply.github.com> Date: Wed, 1 Oct 2025 14:17:41 -0700 Subject: [PATCH 3/4] ActionLoop.destroy doesn't lock on destroy check anymore. --- pykern/pkasyncio.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pykern/pkasyncio.py b/pykern/pkasyncio.py index d622514a..8c60cc7b 100644 --- a/pykern/pkasyncio.py +++ b/pykern/pkasyncio.py @@ -114,8 +114,9 @@ def destroy(self): THREADING: subclasses and external callbacks may call destroy directly. """ - with self.__lock: - self.__destroyed = True + if self.__destroyed: + return + self.__destroyed = True self.__actions.put_nowait((None, None)) def _dispatch_action(self, method, arg): From cdbcf36d84ac26b77463393cc8c23b8845e383a9 Mon Sep 17 00:00:00 2001 From: "Eloise Y." <98988862+eloise-nebula@users.noreply.github.com> Date: Wed, 1 Oct 2025 15:05:36 -0700 Subject: [PATCH 4/4] Put the lock back. Destroy checks for destroyed under lock. --- pykern/pkasyncio.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pykern/pkasyncio.py b/pykern/pkasyncio.py index 8c60cc7b..d50c6016 100644 --- a/pykern/pkasyncio.py +++ b/pykern/pkasyncio.py @@ -114,9 +114,10 @@ def destroy(self): THREADING: subclasses and external callbacks may call destroy directly. """ - if self.__destroyed: - return - self.__destroyed = True + with self.__lock: + if self.__destroyed: + return + self.__destroyed = True self.__actions.put_nowait((None, None)) def _dispatch_action(self, method, arg):