From 7503d34c1f49a96c7ab31b177f6f3a3ffc8ab049 Mon Sep 17 00:00:00 2001 From: Ivan Gotovchits Date: Sun, 28 Jul 2024 09:45:11 -0400 Subject: [PATCH] fix: possible race condition in `AutoRestartTrick` (#1002) * 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. * tries an alternative solution i.e., don't send events if stopped --- src/watchdog/utils/process_watcher.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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")