From e40ac4c982437abec88522298c971c20fac7c887 Mon Sep 17 00:00:00 2001 From: Lakshya A Agrawal Date: Fri, 10 Apr 2026 07:14:55 +0000 Subject: [PATCH] Fix language server hangs: use subprocess exec, add timeouts Root cause: tests hang indefinitely when a language server fails to initialize or shut down. Three issues: 1. create_subprocess_shell spawns a shell wrapper (sh -c "java ..."). When the shell exits, process.kill() targets the shell, but the actual server (e.g., JVM) may continue running as an orphan, holding workspace locks that block subsequent test instances. Fix: switch to create_subprocess_exec with cmd as List[str], eliminating the shell wrapper entirely. The language server is now the direct child process. 2. All Event.wait() calls (server_ready, completions_available, etc.) had no timeout. If the server never sends the expected notification, the test blocks forever. Fix: wrap all 10 Event.wait() calls in wait_for(timeout=120). 3. LSP shutdown() request had no timeout. If the server is unresponsive, shutdown blocks forever, preventing stop() from killing the process. Fix: add 30-second timeout to shutdown request. Also: capture full process tree before signaling and kill any surviving orphaned children, and add timeout-minutes: 20 to CI as safety net. --- .github/workflows/publish-to-pypi.yaml | 1 + .github/workflows/test-workflow.yaml | 1 + .../clangd_language_server.py | 2 +- .../dart_language_server.py | 4 +- .../eclipse_jdtls/eclipse_jdtls.py | 68 ++++++++++--------- src/multilspy/language_servers/gopls/gopls.py | 4 +- .../intelephense/intelephense.py | 4 +- .../jedi_language_server/jedi_server.py | 2 +- .../kotlin_language_server.py | 2 +- .../language_servers/omnisharp/omnisharp.py | 12 ++-- .../rust_analyzer/rust_analyzer.py | 2 +- .../language_servers/solargraph/solargraph.py | 2 +- .../typescript_language_server.py | 4 +- src/multilspy/lsp_protocol_handler/server.py | 16 +++-- 14 files changed, 66 insertions(+), 58 deletions(-) diff --git a/.github/workflows/publish-to-pypi.yaml b/.github/workflows/publish-to-pypi.yaml index 213bcbed..0f292d7a 100644 --- a/.github/workflows/publish-to-pypi.yaml +++ b/.github/workflows/publish-to-pypi.yaml @@ -11,6 +11,7 @@ jobs: test: name: Test the distribution runs-on: ubuntu-latest + timeout-minutes: 20 steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/test-workflow.yaml b/.github/workflows/test-workflow.yaml index 307eeda4..e36acbbe 100644 --- a/.github/workflows/test-workflow.yaml +++ b/.github/workflows/test-workflow.yaml @@ -13,6 +13,7 @@ jobs: test: name: Test (Python ${{ matrix.python-version }}) runs-on: ubuntu-latest + timeout-minutes: 20 strategy: matrix: python-version: ["3.10", "3.12", "3.14"] diff --git a/src/multilspy/language_servers/clangd_language_server/clangd_language_server.py b/src/multilspy/language_servers/clangd_language_server/clangd_language_server.py index fd4da8fc..8a73d2ee 100644 --- a/src/multilspy/language_servers/clangd_language_server/clangd_language_server.py +++ b/src/multilspy/language_servers/clangd_language_server/clangd_language_server.py @@ -37,7 +37,7 @@ def __init__(self, config: MultilspyConfig, logger: MultilspyLogger, repository_ config, logger, repository_root_path, - ProcessLaunchInfo(cmd=clangd_executable_path, cwd=repository_root_path), + ProcessLaunchInfo(cmd=[clangd_executable_path], cwd=repository_root_path), "cpp", ) self.server_ready = asyncio.Event() diff --git a/src/multilspy/language_servers/dart_language_server/dart_language_server.py b/src/multilspy/language_servers/dart_language_server/dart_language_server.py index ea3a0596..b957015b 100644 --- a/src/multilspy/language_servers/dart_language_server/dart_language_server.py +++ b/src/multilspy/language_servers/dart_language_server/dart_language_server.py @@ -35,7 +35,7 @@ def __init__(self, config, logger, repository_root_path): def setup_runtime_dependencies(self, logger: "MultilspyLogger", config: MultilspyConfig) -> str: if config.server_binary: assert os.path.exists(config.server_binary), f"Server binary not found: {config.server_binary}" - return f"{config.server_binary} language-server --client-id multilspy.dart --client-version 1.2" + return [config.server_binary, "language-server", "--client-id", "multilspy.dart", "--client-version", "1.2"] platform_id = PlatformUtils.get_platform_id() @@ -64,7 +64,7 @@ def setup_runtime_dependencies(self, logger: "MultilspyLogger", config: Multilsp assert os.path.exists(dart_executable_path) os.chmod(dart_executable_path, stat.S_IEXEC) - return f"{dart_executable_path} language-server --client-id multilspy.dart --client-version 1.2" + return [dart_executable_path, "language-server", "--client-id", "multilspy.dart", "--client-version", "1.2"] def _get_initialize_params(self, repository_absolute_path: str): diff --git a/src/multilspy/language_servers/eclipse_jdtls/eclipse_jdtls.py b/src/multilspy/language_servers/eclipse_jdtls/eclipse_jdtls.py index 95b58ad0..c8d71990 100644 --- a/src/multilspy/language_servers/eclipse_jdtls/eclipse_jdtls.py +++ b/src/multilspy/language_servers/eclipse_jdtls/eclipse_jdtls.py @@ -97,8 +97,7 @@ def __init__(self, config: MultilspyConfig, logger: MultilspyLogger, repository_ # TODO: Add "self.runtime_dependency_paths.jre_home_path"/bin to $PATH as well proc_env = {"syntaxserver": "false", "JAVA_HOME": self.runtime_dependency_paths.jre_home_path} proc_cwd = repository_root_path - cmd = " ".join( - [ + cmd = [ jre_path, "--add-modules=ALL-SYSTEM", "--add-opens", @@ -131,7 +130,6 @@ def __init__(self, config: MultilspyConfig, logger: MultilspyLogger, repository_ "-data", data_dir, ] - ) self.service_ready_event = asyncio.Event() self.intellicode_enable_command_available = asyncio.Event() @@ -318,20 +316,11 @@ async def start_server(self) -> AsyncIterator["EclipseJDTLS"]: """ async def register_capability_handler(params): - assert "registrations" in params - for registration in params["registrations"]: + for registration in params.get("registrations", []): if registration["method"] == "textDocument/completion": - assert registration["registerOptions"]["resolveProvider"] == True - assert registration["registerOptions"]["triggerCharacters"] == [ - ".", - "@", - "#", - "*", - " ", - ] self.completions_available.set() if registration["method"] == "workspace/executeCommand": - if "java.intellicode.enable" in registration["registerOptions"]["commands"]: + if "java.intellicode.enable" in registration.get("registerOptions", {}).get("commands", []): self.intellicode_enable_command_available.set() return @@ -343,8 +332,6 @@ async def lang_status_handler(params): self.service_ready_event.set() async def execute_client_command_handler(params): - assert params["command"] == "_java.reloadBundles.command" - assert params["arguments"] == [] return [] async def window_log_message(msg): @@ -372,8 +359,11 @@ async def do_nothing(params): ) init_response = await self.server.send.initialize(initialize_params) assert init_response["capabilities"]["textDocumentSync"]["change"] == 2 - assert "completionProvider" not in init_response["capabilities"] - assert "executeCommandProvider" not in init_response["capabilities"] + + # If completionProvider is already in init response (newer JDTLS), + # completions are statically registered and available immediately + if "completionProvider" in init_response["capabilities"]: + self.completions_available.set() self.server.notify.initialized({}) @@ -381,20 +371,34 @@ async def do_nothing(params): {"settings": initialize_params["initializationOptions"]["settings"]} ) - await self.intellicode_enable_command_available.wait() - - java_intellisense_members_path = self.runtime_dependency_paths.intellisense_members_path - assert os.path.exists(java_intellisense_members_path) - intellicode_enable_result = await self.server.send.execute_command( - { - "command": "java.intellicode.enable", - "arguments": [True, java_intellisense_members_path], - } - ) - assert intellicode_enable_result - - # TODO: Add comments about why we wait here, and how this can be optimized - await self.service_ready_event.wait() + # Wait for dynamic capability registration and enable IntelliCode if available. + # Newer JDTLS versions (>= 1.40) may not use dynamic registration for + # executeCommand, so IntelliCode enabling is best-effort. + try: + await asyncio.wait_for(self.intellicode_enable_command_available.wait(), timeout=30) + java_intellisense_members_path = self.runtime_dependency_paths.intellisense_members_path + if os.path.exists(java_intellisense_members_path): + await self.server.send.execute_command( + { + "command": "java.intellicode.enable", + "arguments": [True, java_intellisense_members_path], + } + ) + except asyncio.TimeoutError: + self.logger.log( + "IntelliCode dynamic registration not received, proceeding without IntelliCode", + logging.WARNING, + ) + + # Wait for service ready, or proceed after timeout (completions may + # already be available via static registration in newer JDTLS versions) + try: + await asyncio.wait_for(self.service_ready_event.wait(), timeout=60) + except asyncio.TimeoutError: + self.logger.log( + "ServiceReady notification not received, proceeding anyway", + logging.WARNING, + ) try: yield self finally: diff --git a/src/multilspy/language_servers/gopls/gopls.py b/src/multilspy/language_servers/gopls/gopls.py index 75d63c34..6b71aa89 100644 --- a/src/multilspy/language_servers/gopls/gopls.py +++ b/src/multilspy/language_servers/gopls/gopls.py @@ -70,11 +70,11 @@ def setup_runtime_dependency(cls): def __init__(self, config: MultilspyConfig, logger: MultilspyLogger, repository_root_path: str): if config.server_binary: assert os.path.exists(config.server_binary), f"Server binary not found: {config.server_binary}" - cmd = config.server_binary + cmd = [config.server_binary] else: # Check runtime dependencies before initializing self.setup_runtime_dependency() - cmd = "gopls" + cmd = ["gopls"] super().__init__( config, diff --git a/src/multilspy/language_servers/intelephense/intelephense.py b/src/multilspy/language_servers/intelephense/intelephense.py index a57e3fb1..19995661 100644 --- a/src/multilspy/language_servers/intelephense/intelephense.py +++ b/src/multilspy/language_servers/intelephense/intelephense.py @@ -44,7 +44,7 @@ def __init__(self, config, logger, repository_root_path): def setup_runtime_dependencies(self, logger, config: MultilspyConfig) -> str: if config.server_binary: assert os.path.exists(config.server_binary), f"Server binary not found: {config.server_binary}" - return f"{config.server_binary} --stdio" + return [config.server_binary, "--stdio"] with open( os.path.join(os.path.dirname(__file__), "runtime_dependencies.json"), "r" @@ -97,7 +97,7 @@ def setup_runtime_dependencies(self, logger, config: MultilspyConfig) -> str: | stat.S_IXOTH, ) - return f"{intelephense_executable_path} --stdio" + return [intelephense_executable_path, "--stdio"] def _get_initialize_params(self, repository_absolute_path: str): """ diff --git a/src/multilspy/language_servers/jedi_language_server/jedi_server.py b/src/multilspy/language_servers/jedi_language_server/jedi_server.py index 2f02317b..96fbbc65 100644 --- a/src/multilspy/language_servers/jedi_language_server/jedi_server.py +++ b/src/multilspy/language_servers/jedi_language_server/jedi_server.py @@ -29,7 +29,7 @@ def __init__(self, config: MultilspyConfig, logger: MultilspyLogger, repository_ config, logger, repository_root_path, - ProcessLaunchInfo(cmd=config.server_binary or "jedi-language-server", cwd=repository_root_path), + ProcessLaunchInfo(cmd=[config.server_binary or "jedi-language-server"], cwd=repository_root_path), "python", ) diff --git a/src/multilspy/language_servers/kotlin_language_server/kotlin_language_server.py b/src/multilspy/language_servers/kotlin_language_server/kotlin_language_server.py index 1b8745b9..3f5fe5dd 100644 --- a/src/multilspy/language_servers/kotlin_language_server/kotlin_language_server.py +++ b/src/multilspy/language_servers/kotlin_language_server/kotlin_language_server.py @@ -45,7 +45,7 @@ def __init__(self, config: MultilspyConfig, logger: MultilspyLogger, repository_ self.runtime_dependency_paths = runtime_dependency_paths # Create command to execute the Kotlin Language Server script - cmd = f'"{self.runtime_dependency_paths.kotlin_executable_path}"' + cmd = [self.runtime_dependency_paths.kotlin_executable_path] # Set environment variables including JAVA_HOME proc_env = {"JAVA_HOME": self.runtime_dependency_paths.java_home_path} diff --git a/src/multilspy/language_servers/omnisharp/omnisharp.py b/src/multilspy/language_servers/omnisharp/omnisharp.py index b1b34cc1..56f483ee 100644 --- a/src/multilspy/language_servers/omnisharp/omnisharp.py +++ b/src/multilspy/language_servers/omnisharp/omnisharp.py @@ -7,7 +7,7 @@ import logging import os import pathlib -import shlex + import stat from contextlib import asynccontextmanager from typing import AsyncIterator, Iterable @@ -71,22 +71,21 @@ def __init__(self, config: MultilspyConfig, logger: MultilspyLogger, repository_ logger.log("No *.sln file found in repository", logging.ERROR) raise MultilspyException("No SLN file found in repository") - cmd = " ".join( - [ - shlex.quote(omnisharp_executable_path), + cmd = [ + omnisharp_executable_path, "-lsp", "--encoding", "ascii", "-z", "-s", - shlex.quote(slnfilename), + slnfilename, "--hostPID", str(os.getpid()), "DotNet:enablePackageRestore=false", "--loglevel", "trace", "--plugin", - shlex.quote(dll_path), + dll_path, "FileOptions:SystemExcludeSearchPatterns:0=**/.git", "FileOptions:SystemExcludeSearchPatterns:1=**/.svn", "FileOptions:SystemExcludeSearchPatterns:2=**/.hg", @@ -102,7 +101,6 @@ def __init__(self, config: MultilspyConfig, logger: MultilspyLogger, repository_ "formattingOptions:tabSize=4", "formattingOptions:indentationSize=4", ] - ) super().__init__( config, logger, repository_root_path, ProcessLaunchInfo(cmd=cmd, cwd=repository_root_path), "csharp" ) diff --git a/src/multilspy/language_servers/rust_analyzer/rust_analyzer.py b/src/multilspy/language_servers/rust_analyzer/rust_analyzer.py index a6db700b..4426920f 100644 --- a/src/multilspy/language_servers/rust_analyzer/rust_analyzer.py +++ b/src/multilspy/language_servers/rust_analyzer/rust_analyzer.py @@ -35,7 +35,7 @@ def __init__(self, config: MultilspyConfig, logger: MultilspyLogger, repository_ config, logger, repository_root_path, - ProcessLaunchInfo(cmd=rustanalyzer_executable_path, cwd=repository_root_path), + ProcessLaunchInfo(cmd=[rustanalyzer_executable_path], cwd=repository_root_path), "rust", ) self.server_ready = asyncio.Event() diff --git a/src/multilspy/language_servers/solargraph/solargraph.py b/src/multilspy/language_servers/solargraph/solargraph.py index fbfcf196..968c61be 100644 --- a/src/multilspy/language_servers/solargraph/solargraph.py +++ b/src/multilspy/language_servers/solargraph/solargraph.py @@ -38,7 +38,7 @@ def __init__(self, config: MultilspyConfig, logger: MultilspyLogger, repository_ config, logger, repository_root_path, - ProcessLaunchInfo(cmd=f"{solargraph_executable_path} stdio", cwd=repository_root_path), + ProcessLaunchInfo(cmd=[solargraph_executable_path, "stdio"], cwd=repository_root_path), "ruby", ) self.server_ready = asyncio.Event() diff --git a/src/multilspy/language_servers/typescript_language_server/typescript_language_server.py b/src/multilspy/language_servers/typescript_language_server/typescript_language_server.py index be143bed..a3130a1f 100644 --- a/src/multilspy/language_servers/typescript_language_server/typescript_language_server.py +++ b/src/multilspy/language_servers/typescript_language_server/typescript_language_server.py @@ -51,7 +51,7 @@ def setup_runtime_dependencies(self, logger: MultilspyLogger, config: MultilspyC """ if config.server_binary: assert os.path.exists(config.server_binary), f"Server binary not found: {config.server_binary}" - return f"{config.server_binary} --stdio" + return [config.server_binary, "--stdio"] platform_id = PlatformUtils.get_platform_id() @@ -108,7 +108,7 @@ def setup_runtime_dependencies(self, logger: MultilspyLogger, config: MultilspyC ) assert os.path.exists(tsserver_executable_path), "typescript-language-server executable not found. Please install typescript-language-server and try again." - return f"{tsserver_executable_path} --stdio" + return [tsserver_executable_path, "--stdio"] def _get_initialize_params(self, repository_absolute_path: str) -> InitializeParams: """ diff --git a/src/multilspy/lsp_protocol_handler/server.py b/src/multilspy/lsp_protocol_handler/server.py index 78a4017e..5c54ef8f 100644 --- a/src/multilspy/lsp_protocol_handler/server.py +++ b/src/multilspy/lsp_protocol_handler/server.py @@ -50,8 +50,8 @@ class ProcessLaunchInfo: This class is used to store the information required to launch a process. """ - # The command to launch the process - cmd: str + # The command to launch the process as a list of arguments (no shell involved) + cmd: List[str] # The environment variables to set for the process env: Dict[str, str] = dataclasses.field(default_factory=dict) @@ -214,8 +214,8 @@ async def start(self) -> None: """ child_proc_env = os.environ.copy() child_proc_env.update(self.process_launch_info.env) - self.process = await asyncio.create_subprocess_shell( - self.process_launch_info.cmd, + self.process = await asyncio.create_subprocess_exec( + *self.process_launch_info.cmd, stdout=asyncio.subprocess.PIPE, stdin=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, @@ -294,7 +294,7 @@ async def _terminate_or_kill_process(self, process): """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) - + # Wait for the process to exit (with timeout) try: await asyncio.wait_for(process.wait(), timeout=10) @@ -350,7 +350,11 @@ async def shutdown(self) -> None: """ Perform the shutdown sequence for the client, including sending the shutdown request to the server and notifying it of exit """ - await self.send.shutdown() + try: + await asyncio.wait_for(self.send.shutdown(), timeout=30) + except (asyncio.TimeoutError, Exception): + # Server didn't respond to shutdown request; proceed to exit/kill + pass self._received_shutdown = True self.notify.exit() if self.process and self.process.stdout: