fix(tools): bound env probe subprocess so a Windows inherited pipe can't wedge sessions (#67964)

tests/tools/test_env_probe.py

@@ -155,3 +155,43 @@ def boom(*a, **kw):
         result = env_probe.get_environment_probe_line()
         # Whatever the result is, it must be a string
         assert isinstance(result, str)
+
+
+class TestRunBoundedByTimeout:
+    """``_run`` must return as soon as the *direct* child exits, even when a
+    descendant inherited the captured stdout/stderr handles and outlives it.
+
+    This is the deadlock from #67964: on native Windows a ``pip.exe`` launcher
+    can leave a grandchild holding the captured pipe open, and ``capture_output``
+    reader threads then block far past the timeout while ``_CACHE_LOCK`` is held,
+    wedging every new session. Capturing through temp files removes the reader
+    threads, so a lingering grandchild can't block the parent's ``wait()``.
+
+    Cross-platform repro: the direct child prints ``ok`` and exits immediately
+    after spawning a long-sleeping grandchild that inherits its stdout. With the
+    old pipe-based capture, ``_run`` blocks until the grandchild exits (or hits
+    the 3s timeout and returns ``-1``); with temp-file capture it returns the
+    child's real output within a few milliseconds.
+    """
+
+    def test_returns_before_inheriting_grandchild_exits(self):
+        import time
+
+        grandchild_sleep = 20  # far longer than _run's timeout
+        # Direct child: emit "ok", spawn a detached grandchild that inherits
+        # this process's stdout (no stdout= redirect), then exit right away.
+        child_code = (
+            "import subprocess, sys; "
+            "subprocess.Popen([sys.executable, '-c', "
+            f"'import time; time.sleep({grandchild_sleep})']); "
+            "sys.stdout.write('ok'); sys.stdout.flush()"
+        )
+
+        start = time.monotonic()
+        rc, out, err = env_probe._run([sys.executable, "-c", child_code], timeout=3.0)
+        elapsed = time.monotonic() - start
+
+        # Must not wait on the grandchild, and must not have hit the timeout.
+        assert elapsed < 3.0, f"_run blocked on grandchild for {elapsed:.1f}s"
+        assert rc == 0, f"expected clean exit, got rc={rc} err={err!r}"
+        assert out == "ok"

tools/env_probe.py

@@ -34,6 +34,7 @@
 import shutil
 import subprocess
 import sys
+import tempfile
 import threading
 from typing import Optional
 
@@ -57,21 +58,41 @@ def _run(cmd: list[str], timeout: float = 3.0) -> tuple[int, str, str]:
     """Run a short subprocess.  Returns (returncode, stdout, stderr).
 
     Failures (binary missing, timeout, OSError) return (-1, "", "<reason>").
+
+    Output is captured through temporary files rather than ``capture_output``
+    pipes so ``timeout`` bounds the *whole* call — even on native Windows.  A
+    console-script launcher (e.g. ``pip.exe``) can spawn a descendant that
+    inherits the captured stdout/stderr handles and outlives its parent.  With
+    OS pipes, the reader threads inside ``subprocess.communicate()`` then block
+    until that descendant closes the write end — which the timeout does *not*
+    cover, because killing the direct child leaves the grandchild holding the
+    pipe.  A whole warm probe could hang for ~28 min this way while holding
+    ``_CACHE_LOCK``, wedging every new session's system-prompt build.
+
+    Temp files have no reader threads, so ``wait()`` only ever waits on the
+    direct child; a lingering grandchild holding the handle can't block us, and
+    the probe genuinely fails open on timeout.
     """
     try:
-        result = subprocess.run(
-            cmd,
-            capture_output=True,
-            text=True,
-            timeout=timeout,
-            check=False,
-            stdin=subprocess.DEVNULL,
-        )
-        return result.returncode, (result.stdout or "").strip(), (result.stderr or "").strip()
+        with tempfile.TemporaryFile() as out_f, tempfile.TemporaryFile() as err_f:
+            try:
+                result = subprocess.run(
+                    cmd,
+                    stdout=out_f,
+                    stderr=err_f,
+                    timeout=timeout,
+                    check=False,
+                    stdin=subprocess.DEVNULL,
+                )
+            except subprocess.TimeoutExpired:
+                return -1, "", "timeout"
+            out_f.seek(0)
+            err_f.seek(0)
+            out = out_f.read().decode("utf-8", "replace").strip()
+            err = err_f.read().decode("utf-8", "replace").strip()
+            return result.returncode, out, err
     except FileNotFoundError:
         return -1, "", "not found"
-    except subprocess.TimeoutExpired:
-        return -1, "", "timeout"
     except OSError as exc:
         return -1, "", f"oserror: {exc}"