From 4e78a963b9821f4a6b68395d436bc27e8ad5f17d Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Mon, 11 May 2026 00:08:11 +0200 Subject: [PATCH] Consolidate stdio LS termination logic Apply the process tree signalling functionality that was already implemented in StdioLanguageServer (previously LanguageServerProcess) but not used. Remove the process tree termination from the HLSL language server accordingly. --- .../language_servers/hlsl_language_server.py | 33 ---- src/solidlsp/ls_process.py | 151 ++++++++---------- 2 files changed, 68 insertions(+), 116 deletions(-) diff --git a/src/solidlsp/language_servers/hlsl_language_server.py b/src/solidlsp/language_servers/hlsl_language_server.py index cab201dd..839e0381 100644 --- a/src/solidlsp/language_servers/hlsl_language_server.py +++ b/src/solidlsp/language_servers/hlsl_language_server.py @@ -9,7 +9,6 @@ import pathlib import shutil from typing import Any, cast -import psutil from overrides import override from solidlsp.ls import ( @@ -253,38 +252,6 @@ class HlslLanguageServer(SolidLanguageServer): self.server.notify.initialized({}) - @override - def stop(self, shutdown_timeout: float = 2.0) -> None: - """Kill the shader-language-server process tree before the standard shutdown. - - The base _shutdown() calls process.terminate() directly on the subprocess, - which on Windows with shell=True only kills the cmd.exe wrapper, leaving - the actual shader-language-server binary running as an orphan. We use psutil - to terminate the full process tree first. - """ - process = self.server.process if self.server else None - if process and process.pid and process.returncode is None: - try: - parent = psutil.Process(process.pid) - children = parent.children(recursive=True) - for child in children: - try: - child.terminate() - except (psutil.NoSuchProcess, psutil.AccessDenied): - pass - psutil.wait_procs(children, timeout=2) - for child in children: - try: - if child.is_running(): - child.kill() - except (psutil.NoSuchProcess, psutil.AccessDenied): - pass - except (psutil.NoSuchProcess, psutil.AccessDenied): - pass - except Exception as e: - log.debug(f"Error cleaning up shader-language-server process tree: {e}") - super().stop(shutdown_timeout) - @override def is_ignored_dirname(self, dirname: str) -> bool: """Ignore Unity-specific directories that contain no user-authored shaders.""" diff --git a/src/solidlsp/ls_process.py b/src/solidlsp/ls_process.py index 5142bf9c..247be228 100644 --- a/src/solidlsp/ls_process.py +++ b/src/solidlsp/ls_process.py @@ -189,6 +189,10 @@ class LanguageServerInterface(ABC): :param timeout: the maximum time, in seconds, to wait for the language server to terminate before forcefully killing it (if applicable) """ + if not self.is_running(): + log.debug("Server process not running, skipping shutdown.") + return + self._is_stopping = True self._stop(timeout) @@ -207,6 +211,23 @@ class LanguageServerInterface(ABC): self.notify.exit() log.info("Sent exit notification to server") + def _send_shutdown_in_thread(self) -> None: + """ + Signals shutdown to the server in a separate thread (requests can hang), + and waits for the thread to complete with a timeout. + + :param timeout: timeout, in seconds, to wait for the requests to be handled + """ + log.debug("Sending LSP shutdown request...") + shutdown_thread = threading.Thread(target=self._send_shutdown) + shutdown_thread.daemon = True + shutdown_thread.start() + shutdown_thread.join(timeout=2.0) + if shutdown_thread.is_alive(): + log.debug("LSP shutdown request timed out, proceeding to terminate...") + else: + log.debug("LSP shutdown request completed.") + def _trace(self, src: str, dest: str, message: str | StringDict) -> None: """ Traces LS communication by logging the message with the source and destination of the message @@ -481,76 +502,45 @@ class StdioLanguageServer(LanguageServerInterface): A robust shutdown process designed to terminate cleanly on all platforms, including Windows, by explicitly closing all I/O pipes. """ - if not self.is_running(): - log.debug("Server process not running, skipping shutdown.") - return - log.info(f"Initiating final robust shutdown with a {timeout}s timeout...") process = self.process if process is None: log.debug("Server process is None, cannot shutdown.") return - # --- Main Shutdown Logic --- - # Stage 1: Graceful Termination Request - # Send LSP shutdown and close stdin to signal no more input. try: - log.debug("Sending LSP shutdown request...") - # Use a thread to timeout the LSP shutdown call since it can hang - shutdown_thread = threading.Thread(target=self._send_shutdown) - shutdown_thread.daemon = True - shutdown_thread.start() - shutdown_thread.join(timeout=2.0) # 2 second timeout for LSP shutdown - - if shutdown_thread.is_alive(): - log.debug("LSP shutdown request timed out, proceeding to terminate...") - else: - log.debug("LSP shutdown request completed.") - - if process.stdin and not process.stdin.closed: - process.stdin.close() - log.debug("Stage 1 shutdown complete.") - except Exception as e: - log.debug(f"Exception during graceful shutdown: {e}") - # Ignore errors here, we are proceeding to terminate anyway. - - # Stage 2: Terminate and Wait for Process to Exit - log.debug(f"Terminating process {process.pid}, current status: {process.poll()}") - process.terminate() - - # Stage 3: Wait for process termination with timeout - try: - log.debug(f"Waiting for process {process.pid} to terminate...") - exit_code = process.wait(timeout=timeout) - log.info(f"Language server process terminated successfully with exit code {exit_code}.") - except subprocess.TimeoutExpired: - # If termination failed, forcefully kill the process - log.warning(f"Process {process.pid} termination timed out, killing process forcefully...") - process.kill() + # --- Main Shutdown Logic --- + # Stage 1: Graceful Termination Request + # Send LSP shutdown and close stdin to signal no more input. try: - exit_code = process.wait(timeout=2.0) - log.info(f"Language server process killed successfully with exit code {exit_code}.") + self._send_shutdown_in_thread() + self._safely_close_pipe(process.stdin) + log.debug("Stage 1 shutdown complete.") + except Exception as e: + log.debug(f"Exception during graceful shutdown: {e}") + # Ignore errors here, we are proceeding to terminate anyway. + + # Stage 2: Terminate and Wait for Process to Exit + log.debug(f"Terminating process {process.pid}, current status: {process.poll()}") + self._signal_process_tree(process, terminate=True) + try: + log.debug(f"Waiting for process {process.pid} to terminate...") + exit_code = process.wait(timeout=timeout) + log.info(f"Language server process terminated successfully with exit code {exit_code}.") except subprocess.TimeoutExpired: - log.error(f"Process {process.pid} could not be killed within timeout.") - except Exception as e: - log.error(f"Error during process shutdown: {e}") + # If termination failed, forcefully kill the process + log.warning(f"Process {process.pid} termination timed out, killing process forcefully...") + self._signal_process_tree(process, terminate=False) + try: + exit_code = process.wait(timeout=2.0) + log.info(f"Language server process killed successfully with exit code {exit_code}.") + except subprocess.TimeoutExpired: + log.error(f"Process {process.pid} could not be killed within timeout.") - def _cleanup_process(self, process: subprocess.Popen[bytes]) -> None: - """Clean up a process: close stdin, terminate/kill process, close stdout/stderr.""" - # Close stdin first to prevent deadlocks - # See: https://bugs.python.org/issue35539 - self._safely_close_pipe(process.stdin) - - # Terminate/kill the process if it's still running - if process.returncode is None: - self._terminate_or_kill_process(process) - - # Close stdout and stderr pipes after process has exited - # This is essential to prevent "I/O operation on closed pipe" errors and - # "Event loop is closed" errors during garbage collection - # See: https://bugs.python.org/issue41320 and https://github.com/python/cpython/issues/88050 - self._safely_close_pipe(process.stdout) - self._safely_close_pipe(process.stderr) + except Exception as e: + log.error(f"Error during process shutdown: {e}") + finally: + self.process = None @staticmethod def _safely_close_pipe(pipe: IO[AnyStr] | None) -> None: @@ -561,14 +551,21 @@ class StdioLanguageServer(LanguageServerInterface): except Exception: pass - def _terminate_or_kill_process(self, process: subprocess.Popen[bytes]) -> None: - """Try to terminate the process gracefully, then forcefully if necessary.""" - # First try to terminate the process tree gracefully - self._signal_process_tree(process, terminate=True) - def _signal_process_tree(self, process: subprocess.Popen[bytes], terminate: bool = True) -> None: - """Send signal (terminate or kill) to the process and all its children.""" - signal_method = "terminate" if terminate else "kill" + """ + Sends a signal (terminate or kill) to the given process and all its children. + + :param terminate: if True, signal terminate, otherwise signal kill + """ + + def signal_process(p: subprocess.Popen | psutil.Process) -> None: + try: + if terminate: + p.terminate() + else: + p.kill() + except: + pass # Try to get the parent process parent = None @@ -579,24 +576,12 @@ class StdioLanguageServer(LanguageServerInterface): # If we have the parent process and it's running, signal the entire tree if parent and parent.is_running(): - # Signal children first for child in parent.children(recursive=True): - try: - getattr(child, signal_method)() - except (psutil.NoSuchProcess, psutil.AccessDenied, Exception): - pass - - # Then signal the parent - try: - getattr(parent, signal_method)() - except (psutil.NoSuchProcess, psutil.AccessDenied, Exception): - pass + signal_process(child) + signal_process(parent) + # Otherwise, fall back to direct process signaling else: - # Fall back to direct process signaling - try: - getattr(process, signal_method)() - except Exception: - pass + signal_process(process) def _read_bytes_from_process(self, process, stream, num_bytes) -> bytes: # type: ignore """Read exactly num_bytes from process stdout"""