From bf1e269e58ef618faca5094d7e691afb9da564ba Mon Sep 17 00:00:00 2001 From: Ivan Gotovchits Date: Thu, 10 Aug 2023 13:41:14 +0000 Subject: [PATCH 1/2] fixes a possible race condition in AutoRestartTrick Just a long shot for a failure observed on #998. My hypothesis is that when we stop ProcessWatcher before we restart the process manually, we don't yield to it and immediately kill the process. Next, when the ProcessWatcher thread is woken up, we have to conditions ready - the popen_obj and stopped_event, see the corresponding code, ``` while True: if self.popen_obj.poll() is not None: break if self.stopped_event.wait(timeout=0.1): return ``` And desipte that `stopped_event` is set, we first check for `popen_obj` and trigger the process restart. We can also make the ProcessWatcher logic more robust, by checking if we are stopped before calling the termination callback, e.g., ``` try: if not self.stopped_event.is_set(): self.process_termination_callback() except Exception: logger.exception("Error calling process termination callback") ``` I am not 100% sure about that, as I don't really know what semantics is expected from ProcessWatcher by other users. But at least the AutoRestarter expects this semantics - i.e., a watcher shall not call any events after it was stopped. --- src/watchdog/tricks/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/watchdog/tricks/__init__.py b/src/watchdog/tricks/__init__.py index 65bf074a..c1500d5d 100644 --- a/src/watchdog/tricks/__init__.py +++ b/src/watchdog/tricks/__init__.py @@ -253,6 +253,7 @@ def _stop_process(self): try: if self.process_watcher is not None: self.process_watcher.stop() + self.process_watcher.join() self.process_watcher = None if self.process is not None: From f8c9cb080d11126310c00e360e8b432c2d56153c Mon Sep 17 00:00:00 2001 From: Ivan Gotovchits Date: Thu, 10 Aug 2023 15:07:32 +0000 Subject: [PATCH 2/2] tries an alternative solution i.e., don't send events if stopped --- src/watchdog/tricks/__init__.py | 1 - src/watchdog/utils/process_watcher.py | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/watchdog/tricks/__init__.py b/src/watchdog/tricks/__init__.py index c1500d5d..65bf074a 100644 --- a/src/watchdog/tricks/__init__.py +++ b/src/watchdog/tricks/__init__.py @@ -253,7 +253,6 @@ def _stop_process(self): try: if self.process_watcher is not None: self.process_watcher.stop() - self.process_watcher.join() self.process_watcher = None if self.process is not None: diff --git a/src/watchdog/utils/process_watcher.py b/src/watchdog/utils/process_watcher.py index dd4ece58..46717bc7 100644 --- a/src/watchdog/utils/process_watcher.py +++ b/src/watchdog/utils/process_watcher.py @@ -21,6 +21,7 @@ def run(self): return try: - self.process_termination_callback() + if not self.stopped_event.is_set(): + self.process_termination_callback() except Exception: logger.exception("Error calling process termination callback")