From e933df8662682af98ca323c6c8b44058047da327 Mon Sep 17 00:00:00 2001 From: Tal Einat <532281+taleinat@users.noreply.github.com> Date: Tue, 7 Jun 2022 00:23:46 +0300 Subject: [PATCH 1/9] [watchmedo] Avoid zombie sub-processes when running shell-command without --wait --- changelog.rst | 3 ++- src/watchdog/tricks/__init__.py | 14 +++++++++++- src/watchdog/utils/process_watcher.py | 6 ++---- tests/test_0_watchmedo.py | 31 +++++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 6 deletions(-) diff --git a/changelog.rst b/changelog.rst index 747645cd..0b9245f6 100644 --- a/changelog.rst +++ b/changelog.rst @@ -9,7 +9,8 @@ Unreleased 2022-xx-xx • `full history `__ - [fsevents] Fix flakey test to assert that there are no errors when stopping the emitter. -- [watchmedo] Make auto-restart restart the sub-process if it terminates. (`#896 `__) +- [watchmedo] Make ``auto-restart`` restart the sub-process if it terminates. (`#896 `__) +- [watchmedo] Avoid zombie sub-processes when running ``shell-command`` without ``--wait``. (`#897 `__) - Thanks to our beloved contributors: @samschott, @taleinat 2.1.8 diff --git a/src/watchdog/tricks/__init__.py b/src/watchdog/tricks/__init__.py index 33039240..68fbe929 100644 --- a/src/watchdog/tricks/__init__.py +++ b/src/watchdog/tricks/__init__.py @@ -116,12 +116,14 @@ def __init__(self, shell_command=None, patterns=None, ignore_patterns=None, self.shell_command = shell_command self.wait_for_process = wait_for_process self.drop_during_process = drop_during_process + self.process = None + self._process_watchers = set() def on_any_event(self, event): from string import Template - if self.drop_during_process and self.process and self.process.poll() is None: + if self.drop_during_process and self.is_process_running(): return if event.is_directory: @@ -151,6 +153,16 @@ def on_any_event(self, event): self.process = subprocess.Popen(command, shell=True) if self.wait_for_process: self.process.wait() + else: + process_watcher = ProcessWatcher(self.process, None) + self._process_watchers.add(process_watcher) + def cleanup(): + self._process_watchers.discard(process_watcher) + process_watcher.process_termination_callback = cleanup + process_watcher.start() + + def is_process_running(self): + return self._process_watchers or (self.process is not None and self.process.poll()) class AutoRestartTrick(Trick): diff --git a/src/watchdog/utils/process_watcher.py b/src/watchdog/utils/process_watcher.py index 800d7f8e..f1957648 100644 --- a/src/watchdog/utils/process_watcher.py +++ b/src/watchdog/utils/process_watcher.py @@ -1,5 +1,4 @@ import logging -import time from watchdog.utils import BaseThread @@ -15,11 +14,10 @@ def __init__(self, popen_obj, process_termination_callback): def run(self): while True: - if self.stopped_event.is_set(): - return if self.popen_obj.poll() is not None: break - time.sleep(0.1) + if self.stopped_event.wait(timeout=0.1): + return try: self.process_termination_callback() diff --git a/tests/test_0_watchmedo.py b/tests/test_0_watchmedo.py index d9aaf3fe..d218de61 100644 --- a/tests/test_0_watchmedo.py +++ b/tests/test_0_watchmedo.py @@ -74,6 +74,37 @@ def test_kill_auto_restart(tmpdir, capfd): # assert 'KeyboardInterrupt' in cap.err +def test_shell_command_wait_for_completion(tmpdir, capfd): + from watchdog.events import FileModifiedEvent + from watchdog.tricks import ShellCommandTrick + import shlex + import sys + import time + script = make_dummy_script(tmpdir, n=1) + trick = ShellCommandTrick(shlex.join([sys.executable, script]), wait_for_process=True) + assert not trick.is_process_running() + start_time = time.monotonic() + trick.on_any_event(FileModifiedEvent("foo/bar.baz")) + elapsed = time.monotonic() - start_time + assert not trick.is_process_running() + assert elapsed >= 1 + + +def test_shell_command_subprocess_termination_nowait(tmpdir, capfd): + from watchdog.events import FileModifiedEvent + from watchdog.tricks import ShellCommandTrick + import shlex + import sys + import time + script = make_dummy_script(tmpdir, n=1) + trick = ShellCommandTrick(shlex.join([sys.executable, script]), wait_for_process=False) + assert not trick.is_process_running() + trick.on_any_event(FileModifiedEvent("foo/bar.baz")) + assert trick.is_process_running() + time.sleep(2) + assert not trick.is_process_running() + + def test_auto_restart_subprocess_termination(tmpdir, capfd): from watchdog.tricks import AutoRestartTrick import sys From d05bf7d628ff470dc5434277a680edca4c0dc5b4 Mon Sep 17 00:00:00 2001 From: Tal Einat <532281+taleinat@users.noreply.github.com> Date: Tue, 7 Jun 2022 00:30:00 +0300 Subject: [PATCH 2/9] lint --- src/watchdog/tricks/__init__.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/watchdog/tricks/__init__.py b/src/watchdog/tricks/__init__.py index 68fbe929..7aadcf0d 100644 --- a/src/watchdog/tricks/__init__.py +++ b/src/watchdog/tricks/__init__.py @@ -156,9 +156,8 @@ def on_any_event(self, event): else: process_watcher = ProcessWatcher(self.process, None) self._process_watchers.add(process_watcher) - def cleanup(): - self._process_watchers.discard(process_watcher) - process_watcher.process_termination_callback = cleanup + process_watcher.process_termination_callback = \ + functools.partial(self._process_watchers.discard, process_watcher) process_watcher.start() def is_process_running(self): From 1039437ce18c812b6ce41ab654464e982adffc8b Mon Sep 17 00:00:00 2001 From: Tal Einat <532281+taleinat@users.noreply.github.com> Date: Tue, 7 Jun 2022 00:34:37 +0300 Subject: [PATCH 3/9] reference the issue rather than PR in the changelog --- changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.rst b/changelog.rst index 0b9245f6..7549fec8 100644 --- a/changelog.rst +++ b/changelog.rst @@ -10,7 +10,7 @@ Unreleased - [fsevents] Fix flakey test to assert that there are no errors when stopping the emitter. - [watchmedo] Make ``auto-restart`` restart the sub-process if it terminates. (`#896 `__) -- [watchmedo] Avoid zombie sub-processes when running ``shell-command`` without ``--wait``. (`#897 `__) +- [watchmedo] Avoid zombie sub-processes when running ``shell-command`` without ``--wait``. (`#405 `__) - Thanks to our beloved contributors: @samschott, @taleinat 2.1.8 From 90d62191f28b7b911afd961e6de64e0d335063c3 Mon Sep 17 00:00:00 2001 From: Tal Einat <532281+taleinat@users.noreply.github.com> Date: Tue, 7 Jun 2022 09:54:11 +0300 Subject: [PATCH 4/9] avoid shlex.join which was added in Python 3.8 --- tests/test_0_watchmedo.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_0_watchmedo.py b/tests/test_0_watchmedo.py index d218de61..9602b15c 100644 --- a/tests/test_0_watchmedo.py +++ b/tests/test_0_watchmedo.py @@ -81,7 +81,8 @@ def test_shell_command_wait_for_completion(tmpdir, capfd): import sys import time script = make_dummy_script(tmpdir, n=1) - trick = ShellCommandTrick(shlex.join([sys.executable, script]), wait_for_process=True) + command = " ".join(map(shlex.quote, [sys.executable, script])) + trick = ShellCommandTrick(command, wait_for_process=True) assert not trick.is_process_running() start_time = time.monotonic() trick.on_any_event(FileModifiedEvent("foo/bar.baz")) @@ -97,7 +98,8 @@ def test_shell_command_subprocess_termination_nowait(tmpdir, capfd): import sys import time script = make_dummy_script(tmpdir, n=1) - trick = ShellCommandTrick(shlex.join([sys.executable, script]), wait_for_process=False) + command = " ".join(map(shlex.quote, [sys.executable, script])) + trick = ShellCommandTrick(command, wait_for_process=False) assert not trick.is_process_running() trick.on_any_event(FileModifiedEvent("foo/bar.baz")) assert trick.is_process_running() From 9b58cf3d0cb9ce03988cdef83c6d7d8fdf2a3ace Mon Sep 17 00:00:00 2001 From: Tal Einat <532281+taleinat@users.noreply.github.com> Date: Tue, 7 Jun 2022 09:58:04 +0300 Subject: [PATCH 5/9] increase test wait time --- tests/test_0_watchmedo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_0_watchmedo.py b/tests/test_0_watchmedo.py index 9602b15c..72f49d29 100644 --- a/tests/test_0_watchmedo.py +++ b/tests/test_0_watchmedo.py @@ -103,7 +103,7 @@ def test_shell_command_subprocess_termination_nowait(tmpdir, capfd): assert not trick.is_process_running() trick.on_any_event(FileModifiedEvent("foo/bar.baz")) assert trick.is_process_running() - time.sleep(2) + time.sleep(3) assert not trick.is_process_running() From 2b9b28ab733d2c9769c406604568873a569db0d2 Mon Sep 17 00:00:00 2001 From: Tal Einat <532281+taleinat@users.noreply.github.com> Date: Tue, 7 Jun 2022 10:49:52 +0300 Subject: [PATCH 6/9] try further increasing wait time to get tests passing on Windows --- tests/test_0_watchmedo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_0_watchmedo.py b/tests/test_0_watchmedo.py index 72f49d29..245a3c37 100644 --- a/tests/test_0_watchmedo.py +++ b/tests/test_0_watchmedo.py @@ -103,7 +103,7 @@ def test_shell_command_subprocess_termination_nowait(tmpdir, capfd): assert not trick.is_process_running() trick.on_any_event(FileModifiedEvent("foo/bar.baz")) assert trick.is_process_running() - time.sleep(3) + time.sleep(5) assert not trick.is_process_running() From d2210325f15ee36ddc851279cc44d074bb4ec262 Mon Sep 17 00:00:00 2001 From: Tal Einat <532281+taleinat@users.noreply.github.com> Date: Tue, 7 Jun 2022 10:55:14 +0300 Subject: [PATCH 7/9] fix is_process_running() --- src/watchdog/tricks/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/watchdog/tricks/__init__.py b/src/watchdog/tricks/__init__.py index 7aadcf0d..6e7eaf62 100644 --- a/src/watchdog/tricks/__init__.py +++ b/src/watchdog/tricks/__init__.py @@ -161,7 +161,7 @@ def on_any_event(self, event): process_watcher.start() def is_process_running(self): - return self._process_watchers or (self.process is not None and self.process.poll()) + return self._process_watchers or (self.process is not None and self.process.poll() is None) class AutoRestartTrick(Trick): From b2dd1e5ebb5f12a526f4c7466ba969ecd63eceb8 Mon Sep 17 00:00:00 2001 From: Tal Einat <532281+taleinat@users.noreply.github.com> Date: Tue, 7 Jun 2022 11:08:32 +0300 Subject: [PATCH 8/9] still debugging on Windows... --- tests/test_0_watchmedo.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_0_watchmedo.py b/tests/test_0_watchmedo.py index 245a3c37..4c226dc0 100644 --- a/tests/test_0_watchmedo.py +++ b/tests/test_0_watchmedo.py @@ -65,7 +65,7 @@ def test_kill_auto_restart(tmpdir, capfd): script = make_dummy_script(tmpdir) a = AutoRestartTrick([sys.executable, script]) a.start() - time.sleep(5) + time.sleep(3) a.stop() cap = capfd.readouterr() assert '+++++ 0' in cap.out @@ -87,6 +87,7 @@ def test_shell_command_wait_for_completion(tmpdir, capfd): start_time = time.monotonic() trick.on_any_event(FileModifiedEvent("foo/bar.baz")) elapsed = time.monotonic() - start_time + print(capfd.readouterr()) assert not trick.is_process_running() assert elapsed >= 1 From 18b15d092dd2c27c329a1c3d03a1108d46bd5703 Mon Sep 17 00:00:00 2001 From: Tal Einat <532281+taleinat@users.noreply.github.com> Date: Tue, 7 Jun 2022 11:21:54 +0300 Subject: [PATCH 9/9] apparently Windows doesn't like shell-quoted Python executables --- tests/test_0_watchmedo.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/test_0_watchmedo.py b/tests/test_0_watchmedo.py index 4c226dc0..0f76e986 100644 --- a/tests/test_0_watchmedo.py +++ b/tests/test_0_watchmedo.py @@ -77,11 +77,10 @@ def test_kill_auto_restart(tmpdir, capfd): def test_shell_command_wait_for_completion(tmpdir, capfd): from watchdog.events import FileModifiedEvent from watchdog.tricks import ShellCommandTrick - import shlex import sys import time script = make_dummy_script(tmpdir, n=1) - command = " ".join(map(shlex.quote, [sys.executable, script])) + command = " ".join([sys.executable, script]) trick = ShellCommandTrick(command, wait_for_process=True) assert not trick.is_process_running() start_time = time.monotonic() @@ -92,14 +91,13 @@ def test_shell_command_wait_for_completion(tmpdir, capfd): assert elapsed >= 1 -def test_shell_command_subprocess_termination_nowait(tmpdir, capfd): +def test_shell_command_subprocess_termination_nowait(tmpdir): from watchdog.events import FileModifiedEvent from watchdog.tricks import ShellCommandTrick - import shlex import sys import time script = make_dummy_script(tmpdir, n=1) - command = " ".join(map(shlex.quote, [sys.executable, script])) + command = " ".join([sys.executable, script]) trick = ShellCommandTrick(command, wait_for_process=False) assert not trick.is_process_running() trick.on_any_event(FileModifiedEvent("foo/bar.baz"))