Create _safe_wait_procs() to handle AccessDenied errors

This has been seen on Windows and should be handled.

Author: tysmith

src/ffpuppet/process_tree.py

@@ -9,7 +9,7 @@
 from platform import system
 from subprocess import Popen
 from time import perf_counter, sleep
-from typing import Generator, List, Optional, Tuple
+from typing import Callable, Generator, Iterable, List, Optional, Tuple, cast
 
 try:
     from signal import SIGUSR1, Signals
@@ -40,6 +40,52 @@ def _last_modified(scan_dir: Path) -> Optional[float]:
         return None
 
 
+def _safe_wait_procs(
+    procs: Iterable[Process],
+    timeout: Optional[float] = 0,
+    callback: Optional[Callable[[Process], object]] = None,
+) -> Tuple[List[Process], List[Process]]:
+    """Wrapper for psutil.wait_procs() to avoid AccessDenied.
+    This can be an issue on Windows.
+
+    Args:
+        See psutil.wait_procs().
+
+    Returns:
+        See psutil.wait_procs().
+    """
+    assert timeout is None or timeout >= 0
+
+    deadline = None if timeout is None else perf_counter() + timeout
+    while True:
+        remaining = None if deadline is None else max(deadline - perf_counter(), 0)
+        try:
+            return cast(
+                Tuple[List[Process], List[Process]],
+                wait_procs(procs, timeout=remaining, callback=callback),
+            )
+        except AccessDenied:
+            pass
+        if deadline is not None and deadline <= perf_counter():
+            break
+        sleep(0.25)
+
+    # manually check processes
+    alive: List[Process] = []
+    gone: List[Process] = []
+    for proc in procs:
+        try:
+            if not proc.is_running():
+                gone.append(proc)
+            else:
+                alive.append(proc)
+        except AccessDenied:
+            alive.append(proc)
+        except NoSuchProcess:
+            gone.append(proc)
+    return (gone, alive)
+
+
 def _writing_coverage(procs: List[Process]) -> bool:
     """Check if any processes have open .gcda files.
 
@@ -275,7 +321,7 @@ def terminate(self) -> None:
                 self.parent.wait(timeout=10)
             except (AccessDenied, NoSuchProcess, TimeoutExpired):  # pragma: no cover
                 pass
-            procs = wait_procs(procs, timeout=0)[1]
+            procs = _safe_wait_procs(procs, timeout=0)[1]
 
         use_kill = False
         while procs:
@@ -291,7 +337,7 @@ def terminate(self) -> None:
                 except (AccessDenied, NoSuchProcess):  # pragma: no cover
                     pass
             # wait for processes to terminate
-            procs = wait_procs(procs, timeout=30)[1]
+            procs = _safe_wait_procs(procs, timeout=30)[1]
             if use_kill:
                 break
             use_kill = True
@@ -316,7 +362,7 @@ def wait(self, timeout: int = 300) -> int:
         """
         try:
             exit_code = self.parent.wait(timeout=timeout) or 0
-        except NoSuchProcess:  # pragma: no cover
+        except (AccessDenied, NoSuchProcess):  # pragma: no cover
             # this is triggered sometimes when the process goes away
             exit_code = 0
         return exit_code
@@ -330,4 +376,4 @@ def wait_procs(self, timeout: Optional[float] = 0) -> int:
         Returns:
             Number of processes still alive.
         """
-        return len(wait_procs(self.processes(), timeout=timeout)[1])
+        return len(_safe_wait_procs(self.processes(), timeout=timeout)[1])

src/ffpuppet/test_process_tree.py

@@ -8,12 +8,18 @@
 from pathlib import Path
 from subprocess import Popen
 from time import sleep
+from unittest import mock
 
-from psutil import NoSuchProcess, Process, TimeoutExpired
+from psutil import AccessDenied, NoSuchProcess, Process, TimeoutExpired
 from pytest import mark, raises
 
 from .exceptions import TerminateError
-from .process_tree import ProcessTree, _last_modified, _writing_coverage
+from .process_tree import (
+    ProcessTree,
+    _last_modified,
+    _safe_wait_procs,
+    _writing_coverage,
+)
 
 TREE = Path(__file__).parent / "resources" / "tree.py"
 
@@ -245,3 +251,60 @@ def test_writing_coverage_01(mocker):
     proc.open_files.return_value = (openfile("file.gcda", None),)
     assert _writing_coverage([proc])
     assert proc.open_files.call_count == 1
+
+
+@mark.parametrize(
+    "wait_side_effect, procs, alive_count, gone_count",
+    [
+        # no processes - passthrough
+        ((([], []),), [], 0, 0),
+        # AccessDenied - no procs
+        (AccessDenied(), [], 0, 0),
+        # AccessDenied - alive (is_running check)
+        (
+            AccessDenied(),
+            [mock.Mock(spec_set=Process, is_running=mock.Mock(return_value=True))],
+            1,
+            0,
+        ),
+        # AccessDenied - gone (is_running check)
+        (
+            AccessDenied(),
+            [mock.Mock(spec_set=Process, is_running=mock.Mock(return_value=False))],
+            0,
+            1,
+        ),
+        # AccessDenied - alive
+        (
+            AccessDenied(),
+            [
+                mock.Mock(
+                    spec_set=Process, is_running=mock.Mock(side_effect=AccessDenied())
+                )
+            ],
+            1,
+            0,
+        ),
+        # AccessDenied - gone
+        (
+            AccessDenied(),
+            [
+                mock.Mock(
+                    spec_set=Process,
+                    is_running=mock.Mock(side_effect=NoSuchProcess(pid=1)),
+                )
+            ],
+            0,
+            1,
+        ),
+    ],
+)
+def test_safe_wait_procs_01(mocker, wait_side_effect, procs, alive_count, gone_count):
+    """test ProcessTree._safe_wait_procs()"""
+    mocker.patch("ffpuppet.process_tree.perf_counter", side_effect=count(step=0.25))
+    mocker.patch("ffpuppet.process_tree.sleep", autospec=True)
+    mocker.patch("ffpuppet.process_tree.wait_procs", side_effect=wait_side_effect)
+
+    result = _safe_wait_procs(procs, timeout=1)
+    assert len(result[0]) == gone_count
+    assert len(result[1]) == alive_count