From ce8f9f186058939da54c9d6fead78dfb5d0fd6c3 Mon Sep 17 00:00:00 2001 From: CosmicHourglass <101782820+CosmicHourglass@users.noreply.github.com> Date: Mon, 28 Sep 2026 21:49:16 +0800 Subject: [PATCH] fix: keep s11 and s15 process cleanup working on Windows (#586) Windows has no fcntl, killpg, or SIGKILL, so stopping background processes crashed. Co-authored-by: Cursor --- s11_background_tasks/code.py | 28 ++++++++++++++++++++--- s15_integrated_harness/code.py | 41 +++++++++++++++++++++++++++++----- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/s11_background_tasks/code.py b/s11_background_tasks/code.py index a99f5a1d..ee18f042 100644 --- a/s11_background_tasks/code.py +++ b/s11_background_tasks/code.py @@ -62,8 +62,28 @@ _shell_process_lock = threading.RLock() def _stop_process_group(process: subprocess.Popen): - """Stop processes that remain in the command's original process group.""" - for sig in (signal.SIGTERM, signal.SIGKILL): + """Stop a shell process and its children (cross-platform). + + POSIX uses process-group signals (SIGTERM then SIGKILL). Windows has + neither ``os.killpg`` nor ``signal.SIGKILL``, so it falls back to + ``Popen.terminate()`` / ``Popen.kill()``. + """ + if os.name == "nt": + for stop in (process.terminate, process.kill): + if process.poll() is not None: + return + try: + stop() + except OSError: + return + try: + process.wait(timeout=0.05) + except subprocess.TimeoutExpired: + continue + return + + # SIGKILL is POSIX-only; fall back to SIGTERM on platforms without it. + for sig in (signal.SIGTERM, getattr(signal, "SIGKILL", signal.SIGTERM)): try: os.killpg(process.pid, sig) except (ProcessLookupError, OSError): @@ -90,6 +110,8 @@ signal.signal(signal.SIGTERM, _handle_termination_signal) def _run_bash_process(command: str) -> tuple[str, int | None]: process = None try: + # start_new_session (setsid) exists only on POSIX; skip it on Windows. + popen_kwargs = {} if os.name == "nt" else {"start_new_session": True} process = subprocess.Popen( command, shell=True, @@ -97,7 +119,7 @@ def _run_bash_process(command: str) -> tuple[str, int | None]: stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, errors="replace", - start_new_session=True, + **popen_kwargs, ) with _shell_process_lock: _shell_processes.add(process) diff --git a/s15_integrated_harness/code.py b/s15_integrated_harness/code.py index 7affece2..e9549b2d 100644 --- a/s15_integrated_harness/code.py +++ b/s15_integrated_harness/code.py @@ -20,7 +20,6 @@ Need: pip install anthropic python-dotenv pyyaml + .env with ANTHROPIC_API_KEY import ast import atexit -import fcntl import importlib.util import json import os @@ -37,6 +36,11 @@ from datetime import datetime from dataclasses import dataclass, asdict, field import yaml +try: + import fcntl +except ImportError: # Windows has no fcntl module. + fcntl = None + try: import readline readline.parse_and_bind('set bind-tty-special-chars off') @@ -159,7 +163,10 @@ def task_store_lock(): if depth == 0: TASKS_DIR.mkdir(parents=True, exist_ok=True) handle = TASK_LOCK_PATH.open("a+", encoding="utf-8") - fcntl.flock(handle.fileno(), fcntl.LOCK_EX) + # fcntl.flock gives cross-process locking on POSIX. Windows has no + # fcntl, so it falls back to the in-process threading lock only. + if fcntl is not None: + fcntl.flock(handle.fileno(), fcntl.LOCK_EX) _task_store_state.handle = handle _task_store_state.depth = depth + 1 try: @@ -168,7 +175,8 @@ def task_store_lock(): _task_store_state.depth -= 1 if _task_store_state.depth == 0: handle = _task_store_state.handle - fcntl.flock(handle.fileno(), fcntl.LOCK_UN) + if fcntl is not None: + fcntl.flock(handle.fileno(), fcntl.LOCK_UN) handle.close() del _task_store_state.handle @@ -876,8 +884,26 @@ _shell_process_lock = threading.RLock() def _stop_process_group(process: subprocess.Popen): - """Stop processes that remain in the command's original process group.""" - for sig in (signal.SIGTERM, signal.SIGKILL): + """Stop a shell process and its children (cross-platform). + + POSIX uses process-group signals (SIGTERM then SIGKILL). Windows has + neither ``os.killpg`` nor ``signal.SIGKILL``, so it falls back to + ``Popen.terminate()`` / ``Popen.kill()``. + """ + if os.name == "nt": + for stop in (process.terminate, process.kill): + if process.poll() is not None: + return + try: + stop() + except OSError: + return + try: + process.wait(timeout=0.05) + except subprocess.TimeoutExpired: + continue + return + for sig in (signal.SIGTERM, getattr(signal, "SIGKILL", signal.SIGTERM)): try: os.killpg(process.pid, sig) except ProcessLookupError: @@ -906,10 +932,13 @@ signal.signal(signal.SIGTERM, _handle_termination_signal) def _run_bash_process(command: str, cwd: Path | None = None) -> tuple[str, int | None]: process = None try: + # start_new_session (setsid) exists only on POSIX; skip it on Windows. + popen_kwargs = {} if os.name == "nt" else {"start_new_session": True} process = subprocess.Popen( command, shell=True, cwd=cwd or WORKDIR, stdout=subprocess.PIPE, stderr=subprocess.PIPE, - text=True, errors="replace", start_new_session=True, + text=True, errors="replace", + **popen_kwargs, ) with _shell_process_lock: _shell_processes.add(process)