fix: honor timeout when a grandchild keeps the pipes open

Running a command with a timeout could hang forever, e.g.
local["sh"]["-c", "sleep 30 & wait"](timeout=1).

The timeout thread kills the direct child, but run_proc waits in
proc.communicate(), which returns only on EOF of the pipes. A grandchild
that inherited stdout/stderr keeps them open, so communicate() blocks
indefinitely even though the child is already dead and _timed_out is set.

communicate() now receives the timeout so the wait is bounded; on
expiry the process is killed, reaped, and verify() raises
ProcessTimedOut as usual. Popen-like objects that do not accept a
timeout argument fall back to the previous behavior.

Closes #685

Author: Sanjays2402

plumbum/commands/processes.py

@@ -1,12 +1,13 @@
 from __future__ import annotations
 
-__lazy_modules__ = {"atexit", "contextlib", "heapq", "itertools"}
+__lazy_modules__ = {"atexit", "contextlib", "heapq", "itertools", "subprocess"}
 
 import atexit
 import contextlib
 import enum
 import heapq
 import itertools
+import subprocess
 import sys
 import time
 import typing
@@ -18,7 +19,6 @@
 from plumbum.lib import IS_WIN32
 
 if typing.TYPE_CHECKING:
-    import subprocess
     from collections.abc import Callable, Container, Generator
     from typing import IO, Literal
 
@@ -397,6 +397,34 @@ def _terminate_and_reap(proc: PopenWithAddons[Any], grace: float = 1.0) -> None:
         proc.wait()
 
 
+def _communicate(proc: PopenWithAddons[Any], timeout: float | None) -> tuple[Any, Any]:
+    """Like ``proc.communicate()``, but does not block past the timeout.
+
+    The timeout thread kills *proc* itself, but ``communicate()`` waits for
+    EOF on the pipes, and a grandchild that inherited them keeps them open --
+    so the call would block forever even though *proc* is already dead.
+    Passing the timeout through to ``communicate()`` bounds that wait; the
+    already-set ``_timed_out`` flag then makes ``verify()`` raise
+    ``ProcessTimedOut`` as usual.
+    """
+    if timeout is None:
+        return proc.communicate()
+    try:
+        # Give the timeout thread a moment to kill the process first, so a
+        # normally-terminating process still reports its real exit code.
+        return proc.communicate(timeout=timeout + 1)  # type: ignore[call-arg]
+    except TypeError:
+        # Popen-like objects (remote/session procs) may not accept a timeout.
+        return proc.communicate()
+    except subprocess.TimeoutExpired:
+        proc._timed_out = True  # type: ignore[attr-defined]
+        with contextlib.suppress(Exception):
+            proc.kill()
+        with contextlib.suppress(Exception):
+            proc.wait()
+        return b"", b""
+
+
 # ===================================================================================================
 # run_proc
 # ===================================================================================================
@@ -426,7 +454,7 @@ def run_proc(
     stdout: bytes | str
     stderr: bytes | str
     try:
-        stdout, stderr = proc.communicate()
+        stdout, stderr = _communicate(proc, timeout)
         proc._end_time = time.time()  # type: ignore[attr-defined]
         if not stdout:
             stdout = b""

tests/test_local.py

@@ -621,6 +621,19 @@ def test_timeout(self):
         with pytest.raises(ProcessTimedOut):
             sleep(3, timeout=1)
 
+    @skip_on_windows
+    def test_timeout_with_surviving_grandchild(self):
+        # The timeout thread kills the direct child, but a grandchild that
+        # inherited the pipes keeps them open, so communicate() must not block
+        # waiting for EOF. See issue #685.
+        from plumbum.cmd import sh
+
+        cmd = sh["-c", "sleep 30 & wait"]
+        start = time.time()
+        with pytest.raises(ProcessTimedOut):
+            cmd(timeout=1)
+        assert time.time() - start < 15
+
     @skip_on_windows
     def test_pipe_stderr(self, capfd):
         from plumbum.cmd import cat, head