From 443b5d3ea8d72501aedc7727b6107f89aa7b6a27 Mon Sep 17 00:00:00 2001 From: Predrag Radenkovic Date: Fri, 13 Mar 2026 13:11:36 -0700 Subject: [PATCH 1/4] Properly cancel threads that execute tests before closing the terminal --- module_renderer.py | 4 + plain2code.py | 10 ++- plain2code_exceptions.py | 6 ++ .../actions/prepare_testing_environment.py | 1 + .../actions/run_conformance_tests.py | 1 + render_machine/actions/run_unit_tests.py | 1 + render_machine/render_context.py | 3 + render_machine/render_utils.py | 73 +++++++++++++++---- 8 files changed, 83 insertions(+), 16 deletions(-) diff --git a/module_renderer.py b/module_renderer.py index 5917171..187280e 100644 --- a/module_renderer.py +++ b/module_renderer.py @@ -1,5 +1,6 @@ import argparse import os +import threading import git_utils import plain_file @@ -28,6 +29,7 @@ def __init__( args: argparse.Namespace, run_state: RunState, event_bus: EventBus, + stop_event: threading.Event | None = None, ): self.codeplainAPI = codeplainAPI self.filename = filename @@ -36,6 +38,7 @@ def __init__( self.args = args self.run_state = run_state self.event_bus = event_bus + self.stop_event = stop_event def _ensure_module_folders_exist(self, module_name: str, first_render_frid: str) -> tuple[str, str]: """ @@ -177,6 +180,7 @@ def _build_render_context_for_module( run_state=self.run_state, event_bus=self.event_bus, test_script_timeout=self.args.test_script_timeout, + stop_event=self.stop_event, ) def _render_module( diff --git a/plain2code.py b/plain2code.py index 4ca96fe..ee469d2 100644 --- a/plain2code.py +++ b/plain2code.py @@ -35,6 +35,7 @@ NetworkConnectionError, OutdatedClientVersion, PlainSyntaxError, + RenderCancelledError, ) from plain2code_logger import ( LOGGER_NAME, @@ -50,6 +51,7 @@ from tui.plain2code_tui import Plain2CodeTUI DEFAULT_TEMPLATE_DIRS = importlib.resources.files("standard_template_library") +RENDER_THREAD_SHUTDOWN_TIMEOUT = 0.3 def get_render_range(render_range, plain_source): @@ -199,6 +201,8 @@ def render(args, run_state: RunState, event_bus: EventBus): # noqa: C901 _check_connection(codeplainAPI) + stop_event = threading.Event() + module_renderer = ModuleRenderer( codeplainAPI, args.filename, @@ -207,6 +211,7 @@ def render(args, run_state: RunState, event_bus: EventBus): # noqa: C901 args, run_state, event_bus, + stop_event=stop_event, ) render_error: list[Exception] = [] @@ -214,6 +219,8 @@ def render(args, run_state: RunState, event_bus: EventBus): # noqa: C901 def run_render(): try: module_renderer.render_module() + except RenderCancelledError: + pass # TUI already closed, nothing to report except Exception as e: render_error.append(e) event_bus.publish(RenderFailed(error_message=str(e))) @@ -235,7 +242,8 @@ def run_render(): css_path="styles.css", ) app.run() - render_thread.join(timeout=1) + stop_event.set() + render_thread.join(timeout=RENDER_THREAD_SHUTDOWN_TIMEOUT) if render_error: raise render_error[0] diff --git a/plain2code_exceptions.py b/plain2code_exceptions.py index d75086b..f05cb9e 100644 --- a/plain2code_exceptions.py +++ b/plain2code_exceptions.py @@ -79,3 +79,9 @@ class AmbiguousConfigFileError(Exception): """Raised when a config file is found in both the plain file directory and the current working directory.""" pass + + +class RenderCancelledError(Exception): + """Raised when the render is cancelled by the user closing the TUI.""" + + pass diff --git a/render_machine/actions/prepare_testing_environment.py b/render_machine/actions/prepare_testing_environment.py index 9c9157d..3a685fd 100644 --- a/render_machine/actions/prepare_testing_environment.py +++ b/render_machine/actions/prepare_testing_environment.py @@ -22,6 +22,7 @@ def execute(self, render_context: RenderContext, _previous_action_payload: Any | render_context.verbose, "Testing Environment Preparation", timeout=render_context.test_script_timeout, + stop_event=render_context.stop_event, ) render_context.conformance_tests_running_context.should_prepare_testing_environment = False diff --git a/render_machine/actions/run_conformance_tests.py b/render_machine/actions/run_conformance_tests.py index ad1f683..db789f1 100644 --- a/render_machine/actions/run_conformance_tests.py +++ b/render_machine/actions/run_conformance_tests.py @@ -45,6 +45,7 @@ def execute(self, render_context: RenderContext, _previous_action_payload: Any | "Conformance Tests", frid=render_context.conformance_tests_running_context.current_testing_frid, timeout=render_context.test_script_timeout, + stop_event=render_context.stop_event, ) render_context.script_execution_history.latest_conformance_test_output_path = conformance_tests_temp_file_path render_context.script_execution_history.should_update_script_outputs = True diff --git a/render_machine/actions/run_unit_tests.py b/render_machine/actions/run_unit_tests.py index d36ea60..08d1728 100644 --- a/render_machine/actions/run_unit_tests.py +++ b/render_machine/actions/run_unit_tests.py @@ -25,6 +25,7 @@ def execute(self, render_context: RenderContext, _previous_action_payload: Any | render_context.verbose, "Unit Tests", timeout=render_context.test_script_timeout, + stop_event=render_context.stop_event, ) render_context.script_execution_history.latest_unit_test_output_path = unittests_temp_file_path diff --git a/render_machine/render_context.py b/render_machine/render_context.py index 66522bd..1607728 100644 --- a/render_machine/render_context.py +++ b/render_machine/render_context.py @@ -1,3 +1,4 @@ +import threading from copy import deepcopy from typing import Optional @@ -52,6 +53,7 @@ def __init__( run_state: RunState, event_bus: EventBus, test_script_timeout: Optional[int] = None, + stop_event: Optional[threading.Event] = None, ): self.codeplain_api: CodeplainAPI = codeplain_api self.memory_manager = memory_manager @@ -74,6 +76,7 @@ def __init__( self.verbose = verbose self.run_state = run_state self.event_bus = event_bus + self.stop_event = stop_event self.script_execution_history = ScriptExecutionHistory() self.starting_frid = None self.test_script_timeout = test_script_timeout diff --git a/render_machine/render_utils.py b/render_machine/render_utils.py index 5913e5a..bb536bf 100644 --- a/render_machine/render_utils.py +++ b/render_machine/render_utils.py @@ -1,6 +1,9 @@ +import os +import signal import subprocess import sys import tempfile +import threading import time from typing import Optional @@ -8,6 +11,7 @@ import git_utils import plain_spec from plain2code_console import console +from plain2code_exceptions import RenderCancelledError SCRIPT_EXECUTION_TIMEOUT = 120 TIMEOUT_ERROR_EXIT_CODE = 124 @@ -42,6 +46,21 @@ def print_inputs(render_context, existing_files_content, message): ) +def _kill_process(proc: subprocess.Popen) -> None: + """Kill a process and its entire process group.""" + if sys.platform != "win32": + try: + os.killpg(os.getpgid(proc.pid), signal.SIGTERM) + except (ProcessLookupError, OSError): + proc.terminate() + else: + proc.terminate() + try: + proc.wait(timeout=5) + except subprocess.TimeoutExpired: + proc.kill() + + def execute_script( script: str, scripts_args: list[str], @@ -49,6 +68,7 @@ def execute_script( script_type: str, frid: Optional[str] = None, timeout: Optional[int] = None, + stop_event: Optional[threading.Event] = None, ) -> tuple[int, str, Optional[str]]: temp_file_path = None script_timeout = timeout if timeout is not None else SCRIPT_EXECUTION_TIMEOUT @@ -59,36 +79,56 @@ def execute_script( cmd = ["powershell.exe", "-NoProfile", "-ExecutionPolicy", "Bypass", "-File", script_path] + scripts_args else: cmd = [script_path] + scripts_args + + popen_kwargs = {"start_new_session": True} if sys.platform != "win32" else {} + start_time = time.time() + proc = subprocess.Popen( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + encoding="utf-8", + errors="replace", + **popen_kwargs, + ) + try: - start_time = time.time() - result = subprocess.run( - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - text=True, - encoding="utf-8", - errors="replace", - timeout=script_timeout, - ) + while proc.poll() is None: + if time.time() - start_time >= script_timeout: + partial_stdout = proc.stdout.read() + _kill_process(proc) + exc = subprocess.TimeoutExpired(cmd, script_timeout) + exc.stdout = partial_stdout + raise exc + if stop_event is not None: + stop_event.wait(timeout=0.2) + if stop_event.is_set(): + _kill_process(proc) + raise RenderCancelledError() + else: + time.sleep(0.2) + + stdout = proc.stdout.read() elapsed_time = time.time() - start_time + # Log the info about the script execution if verbose: with tempfile.NamedTemporaryFile( mode="w+", encoding="utf-8", delete=False, suffix=".script_output" ) as temp_file: temp_file.write(f"\n═════════════════════════ {script_type} Script Output ═════════════════════════\n") - temp_file.write(result.stdout) + temp_file.write(stdout) temp_file.write("\n══════════════════════════════════════════════════════════════════════\n") temp_file_path = temp_file.name - if result.returncode != 0: - temp_file.write(f"{script_type} script {script} failed with exit code {result.returncode}.\n") + if proc.returncode != 0: + temp_file.write(f"{script_type} script {script} failed with exit code {proc.returncode}.\n") else: temp_file.write(f"{script_type} script {script} successfully passed.\n") temp_file.write(f"{script_type} script execution time: {elapsed_time:.2f} seconds.\n") console.info(f"[#888888]{script_type} script output stored in: {temp_file_path.strip()}[/#888888]") - if result.returncode != 0: + if proc.returncode != 0: if frid is not None: console.info( f"The {script_type} script for ID {frid} has failed. Initiating the patching mode to automatically correct the discrepancies." @@ -103,7 +143,10 @@ def execute_script( else: console.info(f"[#79FC96]All {script_type} script passed successfully.[/#79FC96]") - return result.returncode, result.stdout, temp_file_path + return proc.returncode, stdout, temp_file_path + + except RenderCancelledError: + raise except subprocess.TimeoutExpired as e: # Store timeout output in a temporary file if verbose: From 39117f2c15c18ef9877bda1ee463177b0acedc34 Mon Sep 17 00:00:00 2001 From: Predrag Radenkovic Date: Fri, 13 Mar 2026 17:35:51 -0700 Subject: [PATCH 2/4] Gracefully kill test process in headless mode --- plain2code.py | 7 ++++++- render_machine/render_utils.py | 7 +++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/plain2code.py b/plain2code.py index ee469d2..c437db7 100644 --- a/plain2code.py +++ b/plain2code.py @@ -2,6 +2,7 @@ import logging import logging.config import os +import signal import sys import threading from pathlib import Path @@ -202,6 +203,7 @@ def render(args, run_state: RunState, event_bus: EventBus): # noqa: C901 _check_connection(codeplainAPI) stop_event = threading.Event() + signal.signal(signal.SIGTERM, lambda _signum, _frame: stop_event.set()) module_renderer = ModuleRenderer( codeplainAPI, @@ -227,7 +229,10 @@ def run_render(): if args.headless: print(f"Render started. Render ID: {run_state.render_id}") - module_renderer.render_module() + try: + module_renderer.render_module() + except RenderCancelledError: + pass return else: render_thread = threading.Thread(target=run_render, daemon=True) diff --git a/render_machine/render_utils.py b/render_machine/render_utils.py index bb536bf..c56c5e7 100644 --- a/render_machine/render_utils.py +++ b/render_machine/render_utils.py @@ -51,7 +51,7 @@ def _kill_process(proc: subprocess.Popen) -> None: if sys.platform != "win32": try: os.killpg(os.getpgid(proc.pid), signal.SIGTERM) - except (ProcessLookupError, OSError): + except OSError: proc.terminate() else: proc.terminate() @@ -61,7 +61,7 @@ def _kill_process(proc: subprocess.Popen) -> None: proc.kill() -def execute_script( +def execute_script( # noqa: C901 script: str, scripts_args: list[str], verbose: bool, @@ -80,7 +80,6 @@ def execute_script( else: cmd = [script_path] + scripts_args - popen_kwargs = {"start_new_session": True} if sys.platform != "win32" else {} start_time = time.time() proc = subprocess.Popen( cmd, @@ -89,7 +88,7 @@ def execute_script( text=True, encoding="utf-8", errors="replace", - **popen_kwargs, + start_new_session=(sys.platform != "win32"), ) try: From e358d5d862ddceac6134c093a9d14d2e01d505b6 Mon Sep 17 00:00:00 2001 From: VitjanZ Date: Tue, 17 Mar 2026 09:26:09 +0100 Subject: [PATCH 3/4] Fixed waiting for EOF by proc.stdout --- render_machine/render_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/render_machine/render_utils.py b/render_machine/render_utils.py index c56c5e7..29727ef 100644 --- a/render_machine/render_utils.py +++ b/render_machine/render_utils.py @@ -94,8 +94,8 @@ def execute_script( # noqa: C901 try: while proc.poll() is None: if time.time() - start_time >= script_timeout: - partial_stdout = proc.stdout.read() _kill_process(proc) + partial_stdout = proc.stdout.read() exc = subprocess.TimeoutExpired(cmd, script_timeout) exc.stdout = partial_stdout raise exc From 8398777383982fc547f1f0faac7c85ac7618f149 Mon Sep 17 00:00:00 2001 From: Predrag Radenkovic Date: Tue, 24 Mar 2026 21:38:49 +0100 Subject: [PATCH 4/4] Properly kill all the whole process group and don't block on normal exit indefinitely if children hold pipe --- plain2code.py | 2 +- render_machine/render_utils.py | 24 +++++++++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/plain2code.py b/plain2code.py index c437db7..b05c8c4 100644 --- a/plain2code.py +++ b/plain2code.py @@ -52,7 +52,7 @@ from tui.plain2code_tui import Plain2CodeTUI DEFAULT_TEMPLATE_DIRS = importlib.resources.files("standard_template_library") -RENDER_THREAD_SHUTDOWN_TIMEOUT = 0.3 +RENDER_THREAD_SHUTDOWN_TIMEOUT = 0.7 def get_render_range(render_range, plain_source): diff --git a/render_machine/render_utils.py b/render_machine/render_utils.py index 29727ef..591be70 100644 --- a/render_machine/render_utils.py +++ b/render_machine/render_utils.py @@ -15,6 +15,9 @@ SCRIPT_EXECUTION_TIMEOUT = 120 TIMEOUT_ERROR_EXIT_CODE = 124 +POLL_INTERVAL_SECONDS = 0.2 +SIGTERM_GRACE_PERIOD_SECONDS = 0.2 +STDOUT_READ_TIMEOUT_SECONDS = 5 def revert_changes_for_frid(render_context): @@ -56,9 +59,15 @@ def _kill_process(proc: subprocess.Popen) -> None: else: proc.terminate() try: - proc.wait(timeout=5) + proc.wait(timeout=SIGTERM_GRACE_PERIOD_SECONDS) except subprocess.TimeoutExpired: - proc.kill() + if sys.platform != "win32": + try: + os.killpg(os.getpgid(proc.pid), signal.SIGKILL) + except OSError: + proc.kill() + else: + proc.kill() def execute_script( # noqa: C901 @@ -100,14 +109,19 @@ def execute_script( # noqa: C901 exc.stdout = partial_stdout raise exc if stop_event is not None: - stop_event.wait(timeout=0.2) + stop_event.wait(timeout=POLL_INTERVAL_SECONDS) if stop_event.is_set(): _kill_process(proc) raise RenderCancelledError() else: - time.sleep(0.2) + time.sleep(POLL_INTERVAL_SECONDS) - stdout = proc.stdout.read() + # Use communicate() with a timeout because child processes may hold the pipe open after the main process exits. + try: + stdout, _ = proc.communicate(timeout=STDOUT_READ_TIMEOUT_SECONDS) + except subprocess.TimeoutExpired: + proc.stdout.close() + stdout = "" elapsed_time = time.time() - start_time # Log the info about the script execution