mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-12 16:15:33 +00:00
Fix manual shutdown triggered by GUI tool/dashboard not cleaning everything up.
Subprocesses were not cleaned up, in particular. * Rename SerenaAgent.shutdown to on_shutdown, because it is really a shutdown handler; it did not perform a shutdown. * Add new method shutdown which is now called from both sites (and which performs the full shutdown) * Manage the process instance used for the dashboard viewer and shut it down explicitly. * Initialise all variables used during shutdown immeditaly (avoiding errors when shutdown is triggered during init)
This commit is contained in:
1 parent
dcc49f19f9
commit
cda321e318
9 files changed
+51
-48
No files matched your search
@@ -8,6 +8,7 @@ Status of the `main` branch. Changes prior to the next official version change w
|
||||
* Fixes:
|
||||
- Fix reactivation of the same project restarting language servers #1280
|
||||
- Fix git commit id in version
|
||||
- Fix manual shutdown triggered by GUI tool/dashboard not cleaning everything up.
|
||||
|
||||
# 1.0.0
|
||||
|
||||
|
||||
@@ -178,7 +178,7 @@ if __name__ == "__main__":
|
||||
run_lsp_tools(lsp_agent)
|
||||
run_backend_independent_tools(lsp_agent)
|
||||
finally:
|
||||
lsp_agent.shutdown()
|
||||
lsp_agent.on_shutdown()
|
||||
|
||||
# JetBrains backend (requires a running IDE)
|
||||
try:
|
||||
@@ -186,6 +186,6 @@ if __name__ == "__main__":
|
||||
try:
|
||||
run_jb_tools(jb_agent)
|
||||
finally:
|
||||
jb_agent.shutdown()
|
||||
jb_agent.on_shutdown()
|
||||
except Exception as e:
|
||||
print(f"\nJetBrains backend not available, skipping: {e}")
|
||||
+33
-13
@@ -6,6 +6,7 @@ import json
|
||||
import multiprocessing
|
||||
import os
|
||||
import platform
|
||||
import signal
|
||||
import subprocess
|
||||
import sys
|
||||
from collections.abc import Callable, Iterator, Sequence
|
||||
@@ -269,6 +270,10 @@ class SerenaAgent:
|
||||
:param memory_log_handler: a MemoryLogHandler instance from which to read log messages; if None, a new one will be created
|
||||
if necessary.
|
||||
"""
|
||||
self._active_project: Project | None = None
|
||||
self._gui_log_viewer: Optional["GuiLogViewer"] = None
|
||||
self._dashboard_viewer_process: multiprocessing.Process | None = None
|
||||
|
||||
self.version = serena_version()
|
||||
|
||||
# obtain serena configuration using the decoupled factory function
|
||||
@@ -277,9 +282,6 @@ class SerenaAgent:
|
||||
# propagate configuration to other components
|
||||
self.serena_config.propagate_settings()
|
||||
|
||||
# project-specific instances, which will be initialized upon project activation
|
||||
self._active_project: Project | None = None
|
||||
|
||||
# determine registered project to be activated (if any)
|
||||
registered_project_to_activate: RegisteredProject | None = (
|
||||
self.serena_config.get_registered_project(project, autoregister=True) if project is not None else None
|
||||
@@ -302,7 +304,6 @@ class SerenaAgent:
|
||||
return memory_log_handler
|
||||
|
||||
# open GUI log window if enabled
|
||||
self._gui_log_viewer: Optional["GuiLogViewer"] = None
|
||||
if self.serena_config.gui_log_window:
|
||||
log.info("Opening GUI window")
|
||||
if platform.system() == "Darwin":
|
||||
@@ -312,7 +313,12 @@ class SerenaAgent:
|
||||
# which uv used as a base, unfortunately)
|
||||
from serena.gui_log_viewer import GuiLogViewer
|
||||
|
||||
self._gui_log_viewer = GuiLogViewer("dashboard", title="Serena Logs", memory_log_handler=get_memory_log_handler())
|
||||
self._gui_log_viewer = GuiLogViewer(
|
||||
"dashboard",
|
||||
title="Serena Logs",
|
||||
memory_log_handler=get_memory_log_handler(),
|
||||
shutdown_handler=lambda: self.shutdown(),
|
||||
)
|
||||
self._gui_log_viewer.start()
|
||||
else:
|
||||
log.debug("GUI window is disabled")
|
||||
@@ -579,8 +585,10 @@ class SerenaAgent:
|
||||
url = self.get_dashboard_url()
|
||||
assert url is not None
|
||||
if SerenaDashboardViewer.is_current_platform_supported():
|
||||
process = multiprocessing.Process(target=self._start_dashboard_viewer_process_function, args=(url, minimized), daemon=True)
|
||||
process.start()
|
||||
self._dashboard_viewer_process = multiprocessing.Process(
|
||||
target=self._start_dashboard_viewer_process_function, args=(url, minimized), daemon=True
|
||||
)
|
||||
self._dashboard_viewer_process.start()
|
||||
else:
|
||||
log.info("Not starting Serena dashboard viewer because the current platform does not support it; using browser-based fallback")
|
||||
if not minimized:
|
||||
@@ -952,23 +960,35 @@ class SerenaAgent:
|
||||
ToolRegistry().print_tool_overview(self._active_tools.tools)
|
||||
|
||||
def __del__(self) -> None:
|
||||
self.shutdown()
|
||||
self.on_shutdown()
|
||||
|
||||
def shutdown(self, timeout: float = 2.0) -> None:
|
||||
def on_shutdown(self, timeout: float = 2.0) -> None:
|
||||
"""
|
||||
Shuts down the agent, freeing resources and stopping background tasks.
|
||||
Shutdown handler of the agent, freeing resources and stopping background tasks.
|
||||
"""
|
||||
# guard against __del__ being called on a partially constructed instance
|
||||
if not hasattr(self, "_active_project"):
|
||||
return
|
||||
log.info("SerenaAgent is shutting down ...")
|
||||
if self._active_project is not None:
|
||||
log.info(f"Shutting down active project '{self._active_project.project_name}' ...")
|
||||
self._active_project.shutdown(timeout=timeout)
|
||||
self._active_project = None
|
||||
if self._gui_log_viewer:
|
||||
log.info("Stopping the GUI log window ...")
|
||||
self._gui_log_viewer.stop()
|
||||
self._gui_log_viewer = None
|
||||
if self._dashboard_viewer_process:
|
||||
log.info("Stopping the dashboard viewer process ...")
|
||||
self._dashboard_viewer_process.terminate()
|
||||
self._dashboard_viewer_process = None
|
||||
|
||||
def shutdown(self) -> None:
|
||||
"""
|
||||
Triggers a hard shutdown of the agent, freeing resources and signalling the process to terminate
|
||||
"""
|
||||
# perform clean-up right away, because kill does not result in normal deletion of the object
|
||||
self.on_shutdown()
|
||||
|
||||
# signal process termination
|
||||
os.kill(os.getpid(), signal.SIGTERM)
|
||||
|
||||
def get_tool_by_name(self, tool_name: str) -> Tool:
|
||||
tool_class = ToolRegistry().get_tool_class_by_name(tool_name)
|
||||
|
||||
+1
-13
@@ -2,7 +2,6 @@ import os
|
||||
import socket
|
||||
import sys
|
||||
import threading
|
||||
from collections.abc import Callable
|
||||
from pathlib import Path
|
||||
from typing import TYPE_CHECKING, Any, Self
|
||||
|
||||
@@ -136,13 +135,11 @@ class SerenaDashboardAPI:
|
||||
memory_log_handler: MemoryLogHandler,
|
||||
tool_names: list[str],
|
||||
agent: "SerenaAgent",
|
||||
shutdown_callback: Callable[[], None] | None = None,
|
||||
tool_usage_stats: ToolUsageStats | None = None,
|
||||
) -> None:
|
||||
self._memory_log_handler = memory_log_handler
|
||||
self._tool_names = tool_names
|
||||
self._agent = agent
|
||||
self._shutdown_callback = shutdown_callback
|
||||
self._app = Flask(__name__)
|
||||
self._tool_usage_stats = tool_usage_stats
|
||||
self._setup_routes()
|
||||
@@ -214,7 +211,7 @@ class SerenaDashboardAPI:
|
||||
|
||||
@self._app.route("/shutdown", methods=["PUT"])
|
||||
def shutdown() -> dict[str, str]:
|
||||
self._shutdown()
|
||||
self._agent.shutdown()
|
||||
return {"status": "shutting down"}
|
||||
|
||||
@self._app.route("/get_available_languages", methods=["GET"])
|
||||
@@ -541,15 +538,6 @@ class SerenaDashboardAPI:
|
||||
serena_version=self._agent.version,
|
||||
)
|
||||
|
||||
def _shutdown(self) -> None:
|
||||
log.info("Shutting down Serena")
|
||||
if self._shutdown_callback:
|
||||
self._shutdown_callback()
|
||||
else:
|
||||
# noinspection PyProtectedMember
|
||||
# noinspection PyUnresolvedReferences
|
||||
os._exit(0)
|
||||
|
||||
def _get_available_languages(self) -> ResponseAvailableLanguages:
|
||||
from solidlsp.ls_config import Language
|
||||
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
# mypy: ignore-errors
|
||||
import logging
|
||||
import os
|
||||
import queue
|
||||
import sys
|
||||
import threading
|
||||
import tkinter as tk
|
||||
import traceback
|
||||
from collections.abc import Callable
|
||||
from enum import Enum, auto
|
||||
from pathlib import Path
|
||||
from typing import Literal
|
||||
@@ -38,6 +38,7 @@ class GuiLogViewer:
|
||||
memory_log_handler: MemoryLogHandler | None = None,
|
||||
width=800,
|
||||
height=600,
|
||||
shutdown_handler: Callable[[], None] | None = None,
|
||||
):
|
||||
"""
|
||||
:param mode: the mode; if "dashboard", run a dashboard with logs and some control options; if "error", run
|
||||
@@ -57,6 +58,7 @@ class GuiLogViewer:
|
||||
self.log_thread = None
|
||||
self.menubar: tk.Menu | None = None
|
||||
self.tool_names = [] # List to store tool names for highlighting
|
||||
self.shutdown_handler = shutdown_handler
|
||||
|
||||
# Define colors for different log levels
|
||||
self.log_colors = {
|
||||
@@ -320,10 +322,8 @@ class GuiLogViewer:
|
||||
self.running = False
|
||||
|
||||
def _shutdown_server(self) -> None:
|
||||
log.info("Shutting down Serena")
|
||||
# noinspection PyUnresolvedReferences
|
||||
# noinspection PyProtectedMember
|
||||
os._exit(0)
|
||||
if self.shutdown_handler is not None:
|
||||
self.shutdown_handler()
|
||||
|
||||
|
||||
class GuiLogViewerHandler(logging.Handler):
|
||||
|
||||
+1
-1
@@ -346,7 +346,7 @@ class SerenaMCPFactory:
|
||||
finally:
|
||||
log.info("MCP server shutting down")
|
||||
if self.agent is not None:
|
||||
self.agent.shutdown()
|
||||
self.agent.on_shutdown()
|
||||
|
||||
def _get_initial_instructions(self) -> str:
|
||||
assert self.agent is not None
|
||||
|
||||
@@ -236,7 +236,7 @@ class TestEffectiveLanguageBackend:
|
||||
try:
|
||||
assert agent.get_language_backend().is_lsp()
|
||||
finally:
|
||||
agent.shutdown(timeout=5)
|
||||
agent.on_shutdown(timeout=5)
|
||||
|
||||
def test_project_overrides_global_backend(self):
|
||||
"""When startup project has language_backend set, it overrides the global."""
|
||||
@@ -247,7 +247,7 @@ class TestEffectiveLanguageBackend:
|
||||
try:
|
||||
assert agent.get_language_backend().is_jetbrains()
|
||||
finally:
|
||||
agent.shutdown(timeout=5)
|
||||
agent.on_shutdown(timeout=5)
|
||||
|
||||
def test_no_project_uses_global_backend(self):
|
||||
"""When no startup project is provided, effective backend is the global one."""
|
||||
@@ -261,7 +261,7 @@ class TestEffectiveLanguageBackend:
|
||||
try:
|
||||
assert agent.get_language_backend() == LanguageBackend.LSP
|
||||
finally:
|
||||
agent.shutdown(timeout=5)
|
||||
agent.on_shutdown(timeout=5)
|
||||
|
||||
def test_activate_project_rejects_backend_mismatch(self):
|
||||
"""Post-init activation of a project with mismatched backend raises ValueError."""
|
||||
@@ -285,7 +285,7 @@ class TestEffectiveLanguageBackend:
|
||||
with pytest.raises(ValueError, match="Cannot activate project"):
|
||||
agent.activate_project_from_path_or_name("jb_proj")
|
||||
finally:
|
||||
agent.shutdown(timeout=5)
|
||||
agent.on_shutdown(timeout=5)
|
||||
|
||||
def test_activate_project_allows_matching_backend(self):
|
||||
"""Post-init activation of a project with matching backend succeeds."""
|
||||
@@ -308,7 +308,7 @@ class TestEffectiveLanguageBackend:
|
||||
# Should not raise
|
||||
agent.activate_project_from_path_or_name("lsp_proj2")
|
||||
finally:
|
||||
agent.shutdown(timeout=5)
|
||||
agent.on_shutdown(timeout=5)
|
||||
|
||||
def test_activate_project_allows_none_backend(self):
|
||||
"""Post-init activation of a project with no backend override succeeds."""
|
||||
@@ -331,7 +331,7 @@ class TestEffectiveLanguageBackend:
|
||||
# Should not raise — None means "inherit session backend"
|
||||
agent.activate_project_from_path_or_name("proj2")
|
||||
finally:
|
||||
agent.shutdown(timeout=5)
|
||||
agent.on_shutdown(timeout=5)
|
||||
|
||||
|
||||
class TestGetConfiguredProjectSerenaFolder:
|
||||
|
||||
@@ -29,13 +29,7 @@ def _make_dashboard(project_languages: list[Language] | None) -> SerenaDashboard
|
||||
if project_languages is not None:
|
||||
project = SimpleNamespace(project_config=SimpleNamespace(languages=project_languages))
|
||||
agent = _DummyAgent(project)
|
||||
return SerenaDashboardAPI(
|
||||
memory_log_handler=_DummyMemoryLogHandler(),
|
||||
tool_names=[],
|
||||
agent=agent,
|
||||
shutdown_callback=None,
|
||||
tool_usage_stats=None,
|
||||
)
|
||||
return SerenaDashboardAPI(memory_log_handler=_DummyMemoryLogHandler(), tool_names=[], agent=agent, tool_usage_stats=None)
|
||||
|
||||
|
||||
def test_available_languages_include_experimental_when_no_active_project():
|
||||
|
||||
@@ -110,7 +110,7 @@ def serena_agent(request: pytest.FixtureRequest, serena_config) -> Iterator[Sere
|
||||
yield agent
|
||||
|
||||
# explicitly shut down to free resources
|
||||
agent.shutdown(timeout=5)
|
||||
agent.on_shutdown(timeout=5)
|
||||
|
||||
|
||||
class TestSerenaAgent:
|
||||
|
||||
Reference in new issue
Block a user