From 3ac66bb8c13bc613f0ce31805b2fbb80f3370a54 Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Thu, 23 Jul 2026 22:12:44 +0000 Subject: [PATCH] fix(ls): don't orphan the language server process when Serena is killed On Linux, a SIGKILLed Serena (or an OOM kill, or a crash) leaves its language server process running: it is launched in its own session so our graceful-shutdown code can clean up the whole tree, but that code never runs if Serena itself dies uncleanly, and no signal from our process group reaches a detached session either. Fix via prctl(PR_SET_PDEATHSIG) at spawn time. Since the process runs under shell=True, the registration alone only protects the intermediate shell; prefixing the command with `exec` makes the shell replace itself with the language server so the same PID (and PDEATHSIG registration) carries through. Fixes #1490 --- CHANGELOG.md | 2 + src/solidlsp/ls_process.py | 13 +++- src/solidlsp/util/subprocess_util.py | 43 +++++++++++-- test/solidlsp/test_pdeathsig.py | 95 ++++++++++++++++++++++++++++ 4 files changed, 147 insertions(+), 6 deletions(-) create mode 100644 test/solidlsp/test_pdeathsig.py diff --git a/CHANGELOG.md b/CHANGELOG.md index e214da84..3d96597c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ Status of the `main` branch. Changes prior to the next official version change w - The `languages` key in project configurations was changed to `language_servers` to better reflect the actual semantics (configurations are automatically migrated) - Fix: glob matching bare `*` and `?` in non-`**` patterns matched across `/`, contradicting documented behaviour #1732 + - Fix: on Linux, a language server process spawned in its own session (the default) is no longer + orphaned when Serena is killed without a chance to shut down cleanly (e.g. SIGKILL, OOM) #1490 * Language Servers: - Allow language server priorities to be configured in `serena_config.yml` (for auto-detection during diff --git a/src/solidlsp/ls_process.py b/src/solidlsp/ls_process.py index b369ff11..4f9b046c 100644 --- a/src/solidlsp/ls_process.py +++ b/src/solidlsp/ls_process.py @@ -2,6 +2,7 @@ import asyncio import json import logging import os +import platform import socket import subprocess import threading @@ -515,10 +516,20 @@ class StdioLanguageServer(LanguageServerInterface): child_proc_env = os.environ.copy() child_proc_env.update(self.process_launch_info.env) - cmd = subprocess_util.convert_shell_cmd(self.process_launch_info.cmd) + # An independent session (its own process group, detached from ours) is what lets a + # SIGKILLed Serena leave the language server behind as an orphan: our own graceful + # shutdown code (subprocess_util.terminate_process_tree_with_kill_fallback) never runs in + # that case, and being in its own session means no signal to our process group reaches it + # either. On Linux, close that gap at spawn time via PR_SET_PDEATHSIG, which needs the + # actual language server process (not the intermediate shell that runs it) to be the one + # holding the registration; see convert_shell_cmd's `exec` prefix. + use_pdeathsig = self.start_independent_lsp_process and platform.system() == "Linux" + cmd = subprocess_util.convert_shell_cmd(self.process_launch_info.cmd, use_exec=use_pdeathsig) log.info("Starting language server process via command: %s", self.process_launch_info.cmd) kwargs = subprocess_util.subprocess_kwargs() kwargs["start_new_session"] = self.start_independent_lsp_process + if use_pdeathsig: + kwargs["preexec_fn"] = subprocess_util.set_pdeathsig_on_parent_exit # the language server is launched with binary (bytes) pipes; the cast is needed because the # presence of platform-specific **kwargs prevents ty from selecting the bytes Popen overload process = cast( diff --git a/src/solidlsp/util/subprocess_util.py b/src/solidlsp/util/subprocess_util.py index e88b9a4f..92a0ce53 100644 --- a/src/solidlsp/util/subprocess_util.py +++ b/src/solidlsp/util/subprocess_util.py @@ -1,5 +1,6 @@ import logging import platform +import signal import subprocess import oslex @@ -7,6 +8,34 @@ import psutil log = logging.getLogger(__name__) +# Resolved once at import time rather than inside the preexec_fn below: preexec_fn runs in the +# forked child before exec(), where only async-signal-safe operations are safe to perform, and an +# import/ctypes.CDLL lookup at that point is not. +_libc = None +if platform.system() == "Linux": + import ctypes + + _libc = ctypes.CDLL("libc.so.6", use_errno=True) +_PR_SET_PDEATHSIG = 1 + + +def set_pdeathsig_on_parent_exit() -> None: + """ + preexec_fn for subprocess.Popen (Linux only, no-op elsewhere): asks the kernel to send this + process SIGTERM if its parent dies for any reason, including SIGKILL, via + prctl(PR_SET_PDEATHSIG). This is the only mechanism that can free the child when the parent + is killed rather than exiting gracefully, since a graceful exit is required to run any + signal-the-tree cleanup code ourselves. + + Only protects the immediate process this is passed to. When launching with shell=True, that + process is the shell, not the program it runs, so the registration alone will not protect a + program the shell forks as a child; see convert_shell_cmd's `exec` prefix, which makes the + shell replace itself with the program instead of forking it, so the same PID (and therefore + the same PDEATHSIG registration, which execve preserves) carries through. + """ + if _libc is not None: + _libc.prctl(_PR_SET_PDEATHSIG, signal.SIGTERM) + def subprocess_kwargs() -> dict: """ @@ -48,18 +77,22 @@ def subprocess_run( return subprocess.run(cmd, check=check, **kwargs) -def convert_shell_cmd(cmd: str | list[str]) -> str: +def convert_shell_cmd(cmd: str | list[str], use_exec: bool = False) -> str: """ Converts a command (specified as a list or string) to a format supported by subprocess calls with shell=True on the current platform, applying necessary escaping and quoting if the command is specified as a list of arguments. :param cmd: the command to convert, specified as a list of arguments + :param use_exec: if True (POSIX only; must not be set on Windows), prefix the command with the shell's + `exec` builtin, so the shell replaces its own process image with the command instead of forking it + as a child. Combined with set_pdeathsig_on_parent_exit, this is what makes the parent-death signal + reach the actual program rather than only the intermediate shell. :return: a suitable representation of the command for subprocess calls on the current platform """ - if isinstance(cmd, list): - return oslex.join(cmd) - else: - return cmd + cmd_str = oslex.join(cmd) if isinstance(cmd, list) else cmd + if use_exec: + cmd_str = f"exec {cmd_str}" + return cmd_str def _signal_process_tree(process: subprocess.Popen[bytes], terminate: bool = True) -> None: diff --git a/test/solidlsp/test_pdeathsig.py b/test/solidlsp/test_pdeathsig.py new file mode 100644 index 00000000..6b5a3090 --- /dev/null +++ b/test/solidlsp/test_pdeathsig.py @@ -0,0 +1,95 @@ +"""Regression test for issue #1490: a SIGKILLed Serena leaves its language server process running. + +``StdioLanguageServer`` starts every LSP process in its own session +(``start_independent_lsp_process`` defaults to True, see ``ls_config.py``) so that our own +graceful-shutdown code can walk and terminate the whole process tree on a clean exit +(``subprocess_util.terminate_process_tree_with_kill_fallback``). That code never runs if Serena +itself is killed (SIGKILL, OOM, crash), and being in its own session means no signal to our +process group reaches the child either, so it survives as a PPID=1 orphan. + +This drives a real ``StdioLanguageServer`` in a child ("fake Serena") process, SIGKILLs that +child, and checks whether the language server process it started is still alive afterwards. +Linux only: the fix is ``prctl(PR_SET_PDEATHSIG)``, which has no equivalent exercised here on +other platforms. +""" + +from __future__ import annotations + +import platform +import subprocess +import sys +import textwrap +import time +import uuid + +import psutil +import pytest + +pytestmark = pytest.mark.skipif(platform.system() != "Linux", reason="PR_SET_PDEATHSIG is Linux-specific") + +_DRIVER_SRC = textwrap.dedent( + """ + import sys + from solidlsp.ls_config import LanguageServerId + from solidlsp.ls_process import StdioLanguageServer + from solidlsp.lsp_protocol_handler.server import ProcessLaunchInfo + + marker = sys.argv[1] + ls = StdioLanguageServer( + process_launch_info=ProcessLaunchInfo( + cmd=[sys.executable, "-c", f"import time; time.sleep(300) # {marker}"], + cwd="/tmp", + ), + ls_id=LanguageServerId.PYTHON, + determine_log_level=lambda _line: 0, + ) + ls._start() + print("READY", flush=True) + time.sleep(600) + """ +) + + +def _find_marked_processes(marker: str) -> list[psutil.Process]: + found = [] + for proc in psutil.process_iter(["cmdline"]): + try: + cmdline = proc.info["cmdline"] or [] + except (psutil.NoSuchProcess, psutil.AccessDenied): + continue + if any(marker in arg for arg in cmdline): + found.append(proc) + return found + + +def test_language_server_process_dies_with_a_sigkilled_serena() -> None: + marker = f"pdeathsig-test-{uuid.uuid4().hex}" + driver = subprocess.Popen( + [sys.executable, "-c", _DRIVER_SRC, marker], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + try: + ready_line = driver.stdout.readline() + assert ready_line.strip() == "READY", f"driver failed to start the language server: {driver.stderr.read()}" + assert _find_marked_processes(marker), "language server process never started" + + driver.kill() # SIGKILL: simulates Serena being killed without a chance to clean up + driver.wait(timeout=5) + + deadline = time.monotonic() + 5 + survivors = _find_marked_processes(marker) + while survivors and time.monotonic() < deadline: + time.sleep(0.2) + survivors = _find_marked_processes(marker) + + assert not survivors, ( + f"language server process(es) {[p.pid for p in survivors]} survived a SIGKILLed Serena " + "(orphaned, will run until manually killed -- issue #1490)" + ) + finally: + for proc in _find_marked_processes(marker): + proc.kill() + if driver.poll() is None: + driver.kill()