diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d8dfa43..83ee56b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ Status of the `main` branch. Changes prior to the next official version change w project list in `serena_config.yml` - Fix: `read_only` restriction in project definition was not applied to base tool set when in single-project context (#1938) +* JetBrains: + - Fix: Concurrent Serena sessions activating different projects at the same time with + `jetbrains_launch_command` set would each independently launch the IDE, racing each other for + the IDE's own config-directory lock; JetBrains IDE launches are now serialized per launch + command and Serena waits for the plugin server to become reachable before proceeding (#1864) + * Language Servers: - Fix: Exceptions raised during `LanguageServerManager.start` did not stop the language server subprocess if it was already started (#1949) diff --git a/src/serena/agent.py b/src/serena/agent.py index 4a2d843f..17ea46c3 100644 --- a/src/serena/agent.py +++ b/src/serena/agent.py @@ -41,7 +41,7 @@ from serena.config.serena_config import ( ToolInclusionDefinition, ) from serena.dashboard import SerenaDashboardAPI, SerenaDashboardTrayManager, SerenaDashboardViewer, open_url_in_browser -from serena.jetbrains import jetbrains_plugin_client +from serena.jetbrains import launch_coordinator as jetbrains_launch_coordinator from serena.ls_manager import LanguageServerManager from serena.memories.memory_manager import MemoryManager from serena.project import Project @@ -1326,18 +1326,13 @@ class SerenaAgent: # for JetBrains mode, search for plugin server and spawn IDE (if not found and launch command provided) elif self.get_language_backend().is_jetbrains(): - try: - client = jetbrains_plugin_client.JetBrainsPluginClient.from_project(project, log_warning=False) + client = jetbrains_launch_coordinator.find_plugin_server(project) + if client is not None: log.info("Found Serena JetBrains Plugin server: %s", client) - except jetbrains_plugin_client.ServerNotFoundError: + else: log.info("Serena JetBrains Plugin server not found for project %s", project.project_name) if self.serena_config.jetbrains_launch_command: - cmd = subprocess_util.convert_shell_cmd([self.serena_config.jetbrains_launch_command, project.project_root]) - log.info("Launching IDE with command: %s", cmd) - p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) - stdout, stderr = p.communicate() - if p.returncode != 0: - log.error(f"Failed to launch JetBrains IDE: {stderr.decode('utf-8')}") + jetbrains_launch_coordinator.launch_and_wait_for_plugin_server(project, self.serena_config.jetbrains_launch_command) def activate_project_from_path_or_name( self, project_root_or_name: str, update_active_modes: bool = True, update_active_tools: bool = True diff --git a/src/serena/jetbrains/launch_coordinator.py b/src/serena/jetbrains/launch_coordinator.py new file mode 100644 index 00000000..690a7d06 --- /dev/null +++ b/src/serena/jetbrains/launch_coordinator.py @@ -0,0 +1,119 @@ +""" +Coordinates the launching of JetBrains IDE instances triggered by Serena's own +`jetbrains_launch_command`, so that concurrent Serena sessions activating different +projects at (nearly) the same time do not race each other into the same IDE launch. +""" + +import hashlib +import logging +import subprocess +import tempfile +import time +from pathlib import Path + +from filelock import FileLock, Timeout + +from serena.jetbrains import jetbrains_plugin_client +from serena.project import Project +from solidlsp.util import subprocess_util + +log = logging.getLogger(__name__) + +LOCK_ACQUIRE_TIMEOUT = 90.0 +""" +How long a session waits for another session's IDE launch (for the same launch command) to +finish before giving up on launching its own instance. +""" + +PLUGIN_SERVER_WAIT_TIMEOUT = 60.0 +"""How long to poll for the plugin server to become reachable after the launch command exits.""" + +PLUGIN_SERVER_POLL_INTERVAL = 1.0 +"""The interval at which we poll for the plugin server while waiting for it to come up.""" + + +def _lock_path_for_launch_command(launch_command: str) -> Path: + """ + :param launch_command: the configured `jetbrains_launch_command` + :return: a stable, cross-process lock file path for the given launch command, so that + concurrent Serena sessions configured with the same launch command serialize on the + same file regardless of which project each of them is activating + """ + digest = hashlib.sha256(launch_command.encode("utf-8")).hexdigest()[:16] + return Path(tempfile.gettempdir()) / f"serena-jetbrains-launch-{digest}.lock" + + +def find_plugin_server(project: Project) -> "jetbrains_plugin_client.JetBrainsPluginClient | None": + """ + :param project: the project to find a matching, already-running plugin server for + :return: the matching plugin server client, or None if none is currently reachable + """ + try: + return jetbrains_plugin_client.JetBrainsPluginClient.from_project(project, log_warning=False) + except jetbrains_plugin_client.ServerNotFoundError: + return None + + +def launch_and_wait_for_plugin_server( + project: Project, + launch_command: str, + lock_acquire_timeout: float = LOCK_ACQUIRE_TIMEOUT, + plugin_server_wait_timeout: float = PLUGIN_SERVER_WAIT_TIMEOUT, + plugin_server_poll_interval: float = PLUGIN_SERVER_POLL_INTERVAL, +) -> None: + """ + Launches the JetBrains IDE for `project` via `launch_command`, serialized across concurrent + Serena sessions that share the same launch command, and waits for the plugin server to + become reachable before returning. + + Without serialization, several sessions activating different projects at once each + independently find no running plugin server and race the same launch command against the + same IDE config directory; only one IDE process wins the JetBrains-side directory lock, and + the others fail with a native modal dialog that never surfaces as a subprocess exit code. + Re-checking for the plugin server both before and after acquiring the lock means every + session but the first either finds the IDE already launching (and waits for it here, rather + than in a failed tool call) or finds it already serving (and returns immediately). + + :param project: the project being activated + :param launch_command: the configured `jetbrains_launch_command` + :param lock_acquire_timeout: how long to wait for another session's launch to finish before + giving up on launching our own instance + :param plugin_server_wait_timeout: how long to poll for the plugin server to become + reachable after the launch command exits + :param plugin_server_poll_interval: the interval at which to poll while waiting + """ + lock = FileLock(str(_lock_path_for_launch_command(launch_command)), timeout=lock_acquire_timeout) + try: + with lock: + if find_plugin_server(project) is not None: + # another session launched the IDE while we were waiting for the lock + return + + cmd = subprocess_util.convert_shell_cmd([launch_command, project.project_root]) + log.info("Launching IDE with command: %s", cmd) + p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) + stdout, stderr = p.communicate() + if p.returncode != 0: + log.error("Failed to launch JetBrains IDE: %s", stderr.decode("utf-8")) + return + + deadline = time.monotonic() + plugin_server_wait_timeout + while True: + if find_plugin_server(project) is not None: + return + if time.monotonic() >= deadline: + log.warning( + "JetBrains IDE launch command exited but no Serena plugin server became reachable " + "for project %s within %.0fs; the IDE may still be starting.", + project.project_name, + plugin_server_wait_timeout, + ) + return + time.sleep(plugin_server_poll_interval) + except Timeout: + log.warning( + "Timed out after %.0fs waiting for another Serena session's JetBrains IDE launch to finish " + "for project %s; not launching a second instance.", + lock_acquire_timeout, + project.project_name, + ) diff --git a/test/serena/test_jetbrains_launch_coordinator.py b/test/serena/test_jetbrains_launch_coordinator.py new file mode 100644 index 00000000..011d697c --- /dev/null +++ b/test/serena/test_jetbrains_launch_coordinator.py @@ -0,0 +1,162 @@ +import threading +import time +from unittest import mock + +from filelock import FileLock + +from serena.jetbrains import jetbrains_plugin_client, launch_coordinator + + +def _make_project(root: str = "/tmp/project", name: str = "project") -> mock.Mock: + project = mock.Mock() + project.project_root = root + project.project_name = name + return project + + +def _fake_process(returncode: int = 0) -> mock.Mock: + process = mock.Mock() + process.communicate.return_value = (b"", b"") + process.returncode = returncode + return process + + +class TestFindPluginServer: + def test_returns_client_when_found(self) -> None: + project = _make_project() + client = mock.Mock() + with mock.patch.object(jetbrains_plugin_client.JetBrainsPluginClient, "from_project", return_value=client) as from_project: + result = launch_coordinator.find_plugin_server(project) + assert result is client + from_project.assert_called_once_with(project, log_warning=False) + + def test_returns_none_when_not_found(self) -> None: + project = _make_project() + with mock.patch.object( + jetbrains_plugin_client.JetBrainsPluginClient, "from_project", side_effect=jetbrains_plugin_client.ServerNotFoundError("nope") + ): + assert launch_coordinator.find_plugin_server(project) is None + + +class TestLaunchAndWaitForPluginServer: + def test_skips_launch_when_a_server_is_already_reachable(self, tmp_path) -> None: + project = _make_project() + client = mock.Mock() + with ( + mock.patch.object(jetbrains_plugin_client.JetBrainsPluginClient, "from_project", return_value=client), + mock.patch.object(launch_coordinator, "_lock_path_for_launch_command", return_value=tmp_path / "lock"), + mock.patch("serena.jetbrains.launch_coordinator.subprocess.Popen") as popen, + ): + launch_coordinator.launch_and_wait_for_plugin_server(project, "pycharm") + popen.assert_not_called() + + def test_launches_once_and_polls_until_the_server_is_reachable(self, tmp_path) -> None: + project = _make_project() + client = mock.Mock() + calls = {"n": 0} + + def from_project(_project: object, log_warning: bool = True) -> object: + calls["n"] += 1 + if calls["n"] <= 3: + raise jetbrains_plugin_client.ServerNotFoundError("not yet") + return client + + with ( + mock.patch.object(jetbrains_plugin_client.JetBrainsPluginClient, "from_project", side_effect=from_project), + mock.patch.object(launch_coordinator, "_lock_path_for_launch_command", return_value=tmp_path / "lock"), + mock.patch("serena.jetbrains.launch_coordinator.subprocess.Popen", return_value=_fake_process()) as popen, + mock.patch("serena.jetbrains.launch_coordinator.time.sleep") as sleep, + ): + launch_coordinator.launch_and_wait_for_plugin_server( + project, "pycharm", plugin_server_wait_timeout=5.0, plugin_server_poll_interval=0.01 + ) + popen.assert_called_once() + # 1 check right after acquiring the lock (raises) + 3 polls (2 raise, the 3rd succeeds) + assert calls["n"] == 4 + assert sleep.call_count == 2 + + def test_gives_up_after_the_wait_timeout_without_raising(self, tmp_path) -> None: + project = _make_project() + with ( + mock.patch.object( + jetbrains_plugin_client.JetBrainsPluginClient, + "from_project", + side_effect=jetbrains_plugin_client.ServerNotFoundError("never"), + ), + mock.patch.object(launch_coordinator, "_lock_path_for_launch_command", return_value=tmp_path / "lock"), + mock.patch("serena.jetbrains.launch_coordinator.subprocess.Popen", return_value=_fake_process()) as popen, + ): + # real, tiny timeout/interval: exercises the deadline branch itself, not a mocked clock + launch_coordinator.launch_and_wait_for_plugin_server( + project, "pycharm", plugin_server_wait_timeout=0.05, plugin_server_poll_interval=0.01 + ) + popen.assert_called_once() + + def test_launch_command_failure_is_logged_and_does_not_poll(self, tmp_path) -> None: + project = _make_project() + with ( + mock.patch.object( + jetbrains_plugin_client.JetBrainsPluginClient, + "from_project", + side_effect=jetbrains_plugin_client.ServerNotFoundError("not yet"), + ) as from_project, + mock.patch.object(launch_coordinator, "_lock_path_for_launch_command", return_value=tmp_path / "lock"), + mock.patch("serena.jetbrains.launch_coordinator.subprocess.Popen", return_value=_fake_process(returncode=1)), + ): + launch_coordinator.launch_and_wait_for_plugin_server(project, "pycharm") + # exactly one check (inside the lock, before launching); a failed launch must not enter the poll loop + assert from_project.call_count == 1 + + def test_lock_acquire_timeout_gives_up_without_launching(self, tmp_path) -> None: + lock_path = tmp_path / "lock" + project = _make_project() + holder = FileLock(str(lock_path)) + holder.acquire() + try: + with ( + mock.patch.object(launch_coordinator, "_lock_path_for_launch_command", return_value=lock_path), + mock.patch("serena.jetbrains.launch_coordinator.subprocess.Popen") as popen, + ): + launch_coordinator.launch_and_wait_for_plugin_server(project, "pycharm", lock_acquire_timeout=0.2) + finally: + holder.release() + popen.assert_not_called() + + def test_concurrent_sessions_serialize_and_launch_only_once(self, tmp_path) -> None: + lock_path = tmp_path / "lock" + ide_started = threading.Event() + popen_calls: list[str] = [] + popen_calls_lock = threading.Lock() + + def fake_popen(cmd: str, **_kwargs: object) -> mock.Mock: + with popen_calls_lock: + popen_calls.append(cmd) + time.sleep(0.05) # simulate the IDE taking a moment to start + ide_started.set() + return _fake_process() + + def from_project(_project: object, log_warning: bool = True) -> object: + if ide_started.is_set(): + return mock.Mock() + raise jetbrains_plugin_client.ServerNotFoundError("not yet") + + with ( + mock.patch.object(jetbrains_plugin_client.JetBrainsPluginClient, "from_project", side_effect=from_project), + mock.patch.object(launch_coordinator, "_lock_path_for_launch_command", return_value=lock_path), + mock.patch("serena.jetbrains.launch_coordinator.subprocess.Popen", side_effect=fake_popen), + ): + threads = [ + threading.Thread( + target=launch_coordinator.launch_and_wait_for_plugin_server, + args=(_make_project(root=f"/tmp/{i}", name=str(i)), "pycharm"), + kwargs={"plugin_server_wait_timeout": 5.0, "plugin_server_poll_interval": 0.01}, + ) + for i in range(2) + ] + for t in threads: + t.start() + for t in threads: + t.join(timeout=10) + assert not t.is_alive() + + assert popen_calls == ["pycharm /tmp/0"] or popen_calls == ["pycharm /tmp/1"]