From be609b625740846dc2750cc13291966cf1216b00 Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Thu, 3 Sep 2026 17:00:04 +0000 Subject: [PATCH] fix(kotlin): give concurrent Serena instances independent LSP storage Two Serena instances activating the same project concurrently launched their Kotlin LSP processes against the same on-disk index storage location (both the modern launcher's --system-path arg and the legacy initializationOptions.storagePath, hardcoded to None), so the second instance's requests were repeatedly cancelled by the first instance's server. DependencyProvider now claims that storage directory with a non-blocking file lock held for the process's lifetime. A single instance, including across restarts, still gets the same directory and keeps its cache; a second concurrent instance falls back to a directory of its own instead of contending for the first one's. Fixes #1966 --- CHANGELOG.md | 6 +++ .../kotlin_language_server.py | 46 ++++++++++++++-- .../kotlin/test_kotlin_dependency_provider.py | 53 +++++++++++++++++++ test/solidlsp/kotlin/test_kotlin_startup.py | 12 +++++ 4 files changed, 114 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 110e9d3d..ec131faa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,12 @@ Status of the `main` branch. Changes prior to the next official version change w silently returned an empty result instead of surfacing the crash. The crash is now detected independently via the `window/logMessage` notification tsserver already sends, and the affected wait now raises instead of reporting success (#1814) + - Fix: two Serena instances activating the same project concurrently launched their Kotlin LSP + processes against the same on-disk index storage location, so the second instance's requests + were repeatedly cancelled by the first instance's server. A Kotlin LSP process now claims that + storage directory via a lock; a single instance (including across restarts) still gets the + same directory, and a second concurrent instance gets a directory of its own instead of + contending for the first one's (#1966) CLI: - Fix `project index-file` command not using only the relevant language server to index the given file (#1965) diff --git a/src/solidlsp/language_servers/kotlin_language_server.py b/src/solidlsp/language_servers/kotlin_language_server.py index df51dfd1..e6584e1d 100644 --- a/src/solidlsp/language_servers/kotlin_language_server.py +++ b/src/solidlsp/language_servers/kotlin_language_server.py @@ -23,6 +23,7 @@ import stat import threading from dataclasses import dataclass +from filelock import FileLock, Timeout from overrides import override from solidlsp.dependency_provider import DownloadedDependency, DownloadedDependencyHashDatabase @@ -117,7 +118,37 @@ class KotlinLanguageServer(SolidLanguageServer): class DependencyProvider(LanguageServerDependencyProviderSinglePath): def __init__(self, custom_settings: SolidLSPSettings.CustomLSSettings, ls_resources_dir: str, project_cache_dir: str): super().__init__(custom_settings, ls_resources_dir) - self._project_cache_dir = project_cache_dir + self._storage_lock: FileLock | None = None + self.storage_dir = self._claim_storage_dir(project_cache_dir) + + def _claim_storage_dir(self, project_cache_dir: str) -> str: + """Claims the on-disk directory this instance's Kotlin LSP process will use for its index storage. + + Tries the shared, deterministic per-project directory first via a non-blocking file lock, so a + single Serena instance keeps reusing its index across restarts (the primary use case, which must + not regress). If another live Serena instance already holds that directory (concurrent sessions + on the same project, see oraios/serena#1966), falls back to a directory unique to this process + instead of two Kotlin LSP processes contending for the same index. + """ + lock = FileLock(f"{project_cache_dir}.lock") + try: + lock.acquire(timeout=0) + except Timeout: + instance_dir = f"{project_cache_dir}-instance-{os.getpid()}" + os.makedirs(instance_dir, exist_ok=True) + log.info( + "Kotlin LSP storage directory %s is in use by another Serena instance; using %s for this instance", + project_cache_dir, + instance_dir, + ) + return instance_dir + self._storage_lock = lock + return project_cache_dir + + def release_storage_lock(self) -> None: + if self._storage_lock is not None: + self._storage_lock.release() + self._storage_lock = None @classmethod def _create_artifact(cls, version: str, platform_id: PlatformId) -> KotlinLSPArtifact: @@ -224,7 +255,7 @@ class KotlinLanguageServer(SolidLanguageServer): platform_id = PlatformUtils.get_platform_id() intellij_launcher_name = "intellij-server.exe" if platform_id.is_windows() else "intellij-server" if os.path.basename(core_path).lower() == intellij_launcher_name: - command.extend(["--system-path", os.path.join(self._project_cache_dir, "kotlin-lsp-system")]) + command.extend(["--system-path", os.path.join(self.storage_dir, "kotlin-lsp-system")]) return command def create_launch_command_env(self) -> dict[str, str]: @@ -243,10 +274,19 @@ class KotlinLanguageServer(SolidLanguageServer): env["JAVA_TOOL_OPTIONS"] = jvm_options return env + @override + def stop(self, shutdown_timeout: float = 2.0) -> None: + super().stop(shutdown_timeout=shutdown_timeout) + if isinstance(self._dependency_provider, self.DependencyProvider): + self._dependency_provider.release_storage_lock() + def _create_base_initialize_params(self) -> dict: """ Returns the initialize params for the Kotlin Language Server. """ + dependency_provider = self._get_dependency_provider() + assert isinstance(dependency_provider, self.DependencyProvider) + storage_dir = dependency_provider.storage_dir root_uri = pathlib.Path(self.repository_root_path).as_uri() initialize_params = { "locale": "en", @@ -459,7 +499,7 @@ class KotlinLanguageServer(SolidLanguageServer): }, "initializationOptions": { "workspaceFolders": [root_uri], - "storagePath": None, + "storagePath": storage_dir, "codegen": {"enabled": False}, "compiler": {"jvm": {"target": "default"}}, "completion": {"snippets": {"enabled": True}}, diff --git a/test/solidlsp/kotlin/test_kotlin_dependency_provider.py b/test/solidlsp/kotlin/test_kotlin_dependency_provider.py index 707a0633..a2c06283 100644 --- a/test/solidlsp/kotlin/test_kotlin_dependency_provider.py +++ b/test/solidlsp/kotlin/test_kotlin_dependency_provider.py @@ -266,6 +266,59 @@ class TestKotlinDependencyProvider: ] assert other_os_provider.create_launch_command() == [other_os_launcher, "--stdio"] + def test_single_instance_keeps_the_deterministic_cache_dir(self, tmp_path: Path) -> None: + """The common case (one Serena instance, possibly restarted) must keep reusing the + same on-disk directory, or the Kotlin LSP loses its index cache on every restart. + """ + provider = _make_provider(tmp_path) + + assert provider.storage_dir == str(tmp_path / "project-cache") + + def test_second_concurrent_instance_gets_its_own_storage_dir(self, tmp_path: Path) -> None: + """Two Serena instances activating the same project concurrently (oraios/serena#1966) + must not be handed the same Kotlin LSP storage directory: the second one falls back to + a directory of its own instead of contending with the first for the same index. + """ + first = _make_provider(tmp_path) + second = _make_provider(tmp_path) + try: + assert first.storage_dir == str(tmp_path / "project-cache") + assert second.storage_dir != first.storage_dir + assert second.storage_dir.startswith(str(tmp_path / "project-cache") + "-instance-") + finally: + first.release_storage_lock() + second.release_storage_lock() + + def test_storage_dir_is_reclaimed_once_the_first_instance_releases_it(self, tmp_path: Path) -> None: + first = _make_provider(tmp_path) + first.release_storage_lock() + + second = _make_provider(tmp_path) + try: + assert second.storage_dir == str(tmp_path / "project-cache") + finally: + second.release_storage_lock() + + def test_concurrent_instances_get_different_system_path_arguments(self, tmp_path: Path) -> None: + launcher = "/path/to/intellij-server" + first = _make_provider(tmp_path, {"ls_path": launcher}) + second = _make_provider(tmp_path, {"ls_path": launcher}) + try: + with patch( + "solidlsp.language_servers.kotlin_language_server.PlatformUtils.get_platform_id", + return_value=PlatformId.LINUX_x64, + ): + first_cmd = first.create_launch_command() + second_cmd = second.create_launch_command() + + assert first_cmd == [launcher, "--stdio", "--system-path", str(tmp_path / "project-cache" / "kotlin-lsp-system")] + assert second_cmd[:2] == [launcher, "--stdio"] + assert second_cmd[2] == "--system-path" + assert second_cmd[3] != first_cmd[3] + finally: + first.release_storage_lock() + second.release_storage_lock() + def test_invalid_version_is_rejected_before_download(self) -> None: with pytest.raises(ValueError, match="dot-separated integers"): KotlinLanguageServer.DependencyProvider._create_artifact("latest", PlatformId.OSX_arm64) diff --git a/test/solidlsp/kotlin/test_kotlin_startup.py b/test/solidlsp/kotlin/test_kotlin_startup.py index 0f6a0345..5a823145 100644 --- a/test/solidlsp/kotlin/test_kotlin_startup.py +++ b/test/solidlsp/kotlin/test_kotlin_startup.py @@ -107,3 +107,15 @@ def test_modern_ready_timeout_warns_and_continues(tmp_path: Path, caplog: Any) - server._start_server() assert "Kotlin LSP did not signal indexing completion within 120s; proceeding anyway" in caplog.text + + +def test_storage_path_is_no_longer_hardcoded_none(tmp_path: Path) -> None: + """A hardcoded None here means every instance shares the LSP's own default index + location; two concurrent instances on the same project then contend for it and the + second fails to index (oraios/serena#1966). + """ + server, _indexing_complete, _intellij_server_ready = _make_server(tmp_path) + + storage_path = server._create_base_initialize_params()["initializationOptions"]["storagePath"] + + assert storage_path == str(tmp_path / "project" / "cache" / "kotlin")