fix(ls): reify the pdeathsig launch ritual into LanguageServerSubprocessLauncher

The previous fix for #1490 depended on four fragments being combined
correctly by hand at the _start() call site: convert_shell_cmd's exec
prefix, the preexec_fn registration, routing through
popen_preserving_pdeathsig, and the Linux/own-session condition.
Nothing enforced their co-occurrence.

Reify "launch a subprocess whose lifecycle is tied to the parent
process (on Linux)" as LanguageServerSubprocessLauncher, a lazily
constructed singleton with a single public method:

    launch(process_launch_info: ProcessLaunchInfo, start_new_session: bool) -> Popen[bytes]

Internalizes the libc/prctl preexec_fn, the exec prefixing, the
platform/own-session gate, and the dedicated long-lived spawner
thread (created lazily, only for own-session launches on Linux). The
singleton is required because the spawner thread it owns must be the
one thread in the process performing pdeathsig-registering fork()
calls; launch() takes start_new_session per call so the instance
itself carries no configuration state.

convert_shell_cmd loses the use_exec flag it gained for this fix: its
other callers (solidlsp.language_servers.common, serena.agent) never
used it, and it is invalid on Windows. The exec prefix now lives
entirely inside the launcher, applied to convert_shell_cmd's plain
output, so it can never be requested without the machinery it depends
on.

StdioLanguageServer._start() now calls
LanguageServerSubprocessLauncher.instance().launch(...) instead of
assembling the four fragments itself.

Adapted test/solidlsp/test_pdeathsig.py: the two existing end-to-end
regression tests (short-lived calling thread, SIGKILLed driver) are
unchanged in behavior since they exercise _start()/start() end-to-end
and now transparently go through the new launcher. Added three
targeted tests: convert_shell_cmd's reverted signature, the
launcher's singleton identity, and that its spawner thread is not
created for a non-own-session launch.

src/solidlsp/ls_process.py

@@ -1,8 +1,6 @@
 import asyncio
 import json
 import logging
-import os
-import platform
 import socket
 import subprocess
 import threading
@@ -11,7 +9,7 @@ from abc import ABC, abstractmethod
 from collections.abc import Callable, Iterable
 from dataclasses import dataclass
 from queue import Empty, Queue
-from typing import IO, Any, AnyStr, cast
+from typing import IO, Any, AnyStr
 
 from sensai.util.string import ToStringMixin
 
@@ -513,42 +511,16 @@ class StdioLanguageServer(LanguageServerInterface):
         return self.process is not None and self.process.returncode is None
 
     def _start(self) -> None:
-        child_proc_env = os.environ.copy()
-        child_proc_env.update(self.process_launch_info.env)
+        log.info("Starting language server process via command: %s", 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
-        # popen_preserving_pdeathsig (rather than a plain subprocess.Popen call) matters specifically
-        # when use_pdeathsig is set: the actual fork() has to happen on a thread that outlives the
-        # calling thread, which may itself be short-lived (e.g. LanguageServerManager's per-server
-        # StartLSThread) -- see its docstring for why a plain Popen call here would defeat the fix
-        # for #1490 in exactly that case.
-        process = cast(
-            "subprocess.Popen[bytes]",
-            subprocess_util.popen_preserving_pdeathsig(
-                cmd,
-                stdout=subprocess.PIPE,
-                stdin=subprocess.PIPE,
-                stderr=subprocess.PIPE,
-                env=child_proc_env,
-                cwd=self.process_launch_info.cwd,
-                shell=True,
-                **kwargs,
-            ),
+        # either. LanguageServerSubprocessLauncher closes that gap on Linux at spawn time via
+        # PR_SET_PDEATHSIG when start_new_session is set.
+        process = subprocess_util.LanguageServerSubprocessLauncher.instance().launch(
+            self.process_launch_info, start_new_session=self.start_independent_lsp_process
         )
         self.process = process
 

src/solidlsp/util/subprocess_util.py

@@ -1,14 +1,19 @@
 import logging
+import os
 import platform
 import queue
 import signal
 import subprocess
 import threading
 from collections.abc import Callable
+from typing import TYPE_CHECKING, Any, cast
 
 import oslex
 import psutil
 
+if TYPE_CHECKING:
+    from solidlsp.lsp_protocol_handler.server import ProcessLaunchInfo
+
 log = logging.getLogger(__name__)
 
 # Resolved once at import time rather than inside the preexec_fn below: preexec_fn runs in the
@@ -32,14 +37,15 @@ def set_pdeathsig_on_parent_exit() -> None:
 
     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.
+    program the shell forks as a child; see LanguageServerSubprocessLauncher'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.
 
-    Warning this function's caller (popen_preserving_pdeathsig below) exists to guard against: per
-    `man 2 prctl`, PR_SET_PDEATHSIG's "parent" is specifically the OS THREAD that called fork() to
-    create this process, not the parent process as a whole -- if that thread terminates while the
-    rest of the parent process (e.g. its main thread) keeps running, the death signal fires anyway.
+    Warning this function's caller (LanguageServerSubprocessLauncher below) exists to guard
+    against: per `man 2 prctl`, PR_SET_PDEATHSIG's "parent" is specifically the OS THREAD that
+    called fork() to create this process, not the parent process as a whole -- if that thread
+    terminates while the rest of the parent process (e.g. its main thread) keeps running, the death
+    signal fires anyway.
     """
     if _libc is not None:
         _libc.prctl(_PR_SET_PDEATHSIG, signal.SIGTERM)
@@ -51,8 +57,8 @@ class _PDeathSigSpawner:
     running daemon thread, so the "parent thread" the kernel ties the registration to is never a
     short-lived one.
 
-    Why this exists: StdioLanguageServer._start() (the sole caller of popen_preserving_pdeathsig)
-    is very often invoked from a thread that is *intentionally* short-lived. For example,
+    Why this exists: LanguageServerSubprocessLauncher.launch() (the sole caller of this class) is
+    very often invoked from a thread that is *intentionally* short-lived. For example,
     LanguageServerManager.from_languages spawns one StartLSThread per language server; each one
     calls language_server.start() (-> ._start(), where Popen() happens) and then simply returns,
     terminating moments after Popen() itself returns. Because PR_SET_PDEATHSIG's delivery is tied
@@ -87,33 +93,6 @@ class _PDeathSigSpawner:
         return process
 
 
-class _SpawnerHolder:
-    instance: "_PDeathSigSpawner | None" = None
-
-
-_pdeathsig_spawner_holder = _SpawnerHolder()
-_pdeathsig_spawner_lock = threading.Lock()
-
-
-def popen_preserving_pdeathsig(cmd: str | list[str], **kwargs) -> subprocess.Popen:
-    """
-    subprocess.Popen wrapper for spawns that pass preexec_fn=set_pdeathsig_on_parent_exit.
-
-    On Linux, delegates the actual Popen() call to a single dedicated daemon thread (see
-    _PDeathSigSpawner) instead of performing it on the calling thread directly, so that the
-    PR_SET_PDEATHSIG registration set up by the preexec_fn remains tied to a thread that lives as
-    long as the process does. On any other platform (or if no preexec_fn is passed), this is
-    exactly equivalent to calling subprocess.Popen(cmd, **kwargs) directly.
-    """
-    if platform.system() != "Linux" or kwargs.get("preexec_fn") is None:
-        return subprocess.Popen(cmd, **kwargs)
-    with _pdeathsig_spawner_lock:
-        if _pdeathsig_spawner_holder.instance is None:
-            _pdeathsig_spawner_holder.instance = _PDeathSigSpawner()
-        spawner = _pdeathsig_spawner_holder.instance
-    return spawner.spawn(lambda: subprocess.Popen(cmd, **kwargs))
-
-
 def subprocess_kwargs() -> dict:
     """
     Returns a dictionary of keyword arguments for subprocess calls, adding platform-specific
@@ -154,22 +133,116 @@ def subprocess_run(
     return subprocess.run(cmd, check=check, **kwargs)
 
 
-def convert_shell_cmd(cmd: str | list[str], use_exec: bool = False) -> str:
+def convert_shell_cmd(cmd: str | list[str]) -> 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
     """
-    cmd_str = oslex.join(cmd) if isinstance(cmd, list) else cmd
-    if use_exec:
-        cmd_str = f"exec {cmd_str}"
-    return cmd_str
+    return oslex.join(cmd) if isinstance(cmd, list) else cmd
+
+
+class LanguageServerSubprocessLauncher:
+    """
+    Singleton that launches a language server subprocess whose lifecycle is tied to this process
+    (on Linux, when requested via ``start_new_session``). This reifies, as one unit, several
+    pieces that only work correctly when combined and would otherwise have to be assembled by hand
+    at every call site: the `exec` prefixing of the shell command, the libc/prctl preexec_fn that
+    registers PR_SET_PDEATHSIG, the platform/own-session gate that decides whether any of this
+    applies, and the dedicated long-lived spawner thread that the PR_SET_PDEATHSIG registration
+    requires (see _PDeathSigSpawner: its "parent" is the calling OS *thread*, not the process, so a
+    short-lived calling thread would defeat it).
+
+    The class is a singleton because the spawner thread it owns must be a single, permanently
+    running thread for the entire process: PR_SET_PDEATHSIG's registration is tied to whichever OS
+    thread performs the fork(), so there can only be one such thread doing pdeathsig-registering
+    spawns if the registration is to reliably outlive the caller. ``launch()`` takes
+    ``start_new_session`` per call and the instance otherwise carries no per-call configuration, so
+    sharing one instance across all callers is safe.
+    """
+
+    _instance: "LanguageServerSubprocessLauncher | None" = None
+    _instance_lock = threading.Lock()
+
+    def __init__(self) -> None:
+        # Created lazily (see _spawn_with_pdeathsig): the dedicated spawner thread is only needed
+        # for own-session launches on Linux, which not every caller requests.
+        self._spawner: _PDeathSigSpawner | None = None
+        self._spawner_lock = threading.Lock()
+
+    @classmethod
+    def instance(cls) -> "LanguageServerSubprocessLauncher":
+        if cls._instance is None:
+            with cls._instance_lock:
+                if cls._instance is None:
+                    cls._instance = cls()
+        return cls._instance
+
+    def launch(self, process_launch_info: "ProcessLaunchInfo", start_new_session: bool) -> subprocess.Popen[bytes]:
+        """
+        Launches a language server process from ``process_launch_info``.
+
+        :param process_launch_info: the command, environment and working directory to launch with
+        :param start_new_session: whether to start the process in its own session (own process
+            group, detached from ours). This is what leaves an orphan behind if we are SIGKILLed
+            without a chance to clean up, so on Linux, requesting it also arms
+            prctl(PR_SET_PDEATHSIG) on the actual language server process (not the intermediate
+            shell -- see the `exec` prefix below), ensuring the kernel sends it SIGTERM if this
+            process dies for any reason. On other platforms, this parameter only controls the
+            process-group behavior; there is no equivalent kernel-level protection available.
+        :return: the started process, with binary (bytes) stdio pipes
+        """
+        child_proc_env = os.environ.copy()
+        child_proc_env.update(process_launch_info.env)
+
+        use_pdeathsig = start_new_session and platform.system() == "Linux"
+        cmd = convert_shell_cmd(process_launch_info.cmd)
+        if use_pdeathsig:
+            # Makes the shell replace its own process image with the program (execve) instead of
+            # forking it as a child, so the PID -- and therefore the PR_SET_PDEATHSIG registration
+            # below, which execve preserves -- carries through to the actual language server
+            # process rather than protecting only the intermediate shell.
+            cmd = f"exec {cmd}"
+
+        kwargs: dict[str, Any] = subprocess_kwargs()
+        kwargs["start_new_session"] = start_new_session
+        if use_pdeathsig:
+            kwargs["preexec_fn"] = set_pdeathsig_on_parent_exit
+
+        def do_popen() -> subprocess.Popen[bytes]:
+            # 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
+            return cast(
+                "subprocess.Popen[bytes]",
+                subprocess.Popen(
+                    cmd,
+                    stdout=subprocess.PIPE,
+                    stdin=subprocess.PIPE,
+                    stderr=subprocess.PIPE,
+                    env=child_proc_env,
+                    cwd=process_launch_info.cwd,
+                    shell=True,
+                    **kwargs,
+                ),
+            )
+
+        # Funneling the fork() itself through the dedicated spawner thread (rather than calling
+        # Popen() on the calling thread directly) matters specifically when use_pdeathsig is set:
+        # the actual fork() has to happen on a thread that outlives the calling thread, which may
+        # itself be short-lived (e.g. LanguageServerManager's per-server StartLSThread) -- see
+        # _PDeathSigSpawner's docstring for why a plain Popen() call here would defeat the fix for
+        # #1490 in exactly that case. When pdeathsig isn't needed, calling Popen() directly is
+        # exactly equivalent and avoids spinning up the spawner thread for no reason.
+        if not use_pdeathsig:
+            return do_popen()
+        with self._spawner_lock:
+            if self._spawner is None:
+                self._spawner = _PDeathSigSpawner()
+            spawner = self._spawner
+        return spawner.spawn(do_popen)
 
 
 def _signal_process_tree(process: subprocess.Popen[bytes], terminate: bool = True) -> None:

test/solidlsp/test_pdeathsig.py

@@ -15,6 +15,7 @@ other platforms.
 
 from __future__ import annotations
 
+import inspect
 import platform
 import subprocess
 import sys
@@ -25,6 +26,9 @@ import uuid
 import psutil
 import pytest
 
+from solidlsp.lsp_protocol_handler.server import ProcessLaunchInfo
+from solidlsp.util.subprocess_util import LanguageServerSubprocessLauncher, convert_shell_cmd
+
 pytestmark = pytest.mark.skipif(platform.system() != "Linux", reason="PR_SET_PDEATHSIG is Linux-specific")
 
 _DRIVER_SRC = textwrap.dedent(
@@ -134,6 +138,44 @@ def test_language_server_process_survives_a_short_lived_calling_thread() -> None
             driver.kill()
 
 
+def test_convert_shell_cmd_has_no_lifecycle_concern() -> None:
+    """
+    convert_shell_cmd is a pure command-formatting utility; its other callers (e.g.
+    solidlsp.language_servers.common, serena.agent) have no use for the `exec` prefixing that
+    PR_SET_PDEATHSIG needs, and prefixing with `exec` is invalid on Windows. That concern now
+    lives entirely inside LanguageServerSubprocessLauncher, applied to this function's output, so
+    it can never be requested without the preexec_fn/dedicated-thread machinery it depends on.
+    """
+    assert "use_exec" not in inspect.signature(convert_shell_cmd).parameters
+    assert convert_shell_cmd(["python3", "-c", "pass"]) == "python3 -c pass"
+
+
+def test_language_server_subprocess_launcher_is_a_singleton() -> None:
+    assert LanguageServerSubprocessLauncher.instance() is LanguageServerSubprocessLauncher.instance()
+
+
+def test_language_server_subprocess_launcher_spawner_thread_is_lazy() -> None:
+    """
+    The dedicated spawner thread is only needed for own-session launches on Linux; a fresh
+    launcher instance that has only ever performed a non-own-session launch should not have
+    created it.
+    """
+    launcher = LanguageServerSubprocessLauncher()
+    assert launcher._spawner is None
+
+    process = launcher.launch(
+        ProcessLaunchInfo(cmd=[sys.executable, "-c", "print('hi')"], cwd="/tmp"),
+        start_new_session=False,
+    )
+    try:
+        process.wait(timeout=5)
+    finally:
+        if process.poll() is None:
+            process.kill()
+
+    assert launcher._spawner is None
+
+
 def test_language_server_process_dies_with_a_sigkilled_serena() -> None:
     marker = f"pdeathsig-test-{uuid.uuid4().hex}"
     driver = subprocess.Popen(