mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-13 00:15:05 +00:00
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
This commit is contained in:
4 files changed
+147
-6
No files matched your search
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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()
|
||||
Reference in new issue
Block a user