fix(profiles): release routed log locks before deletion
This commit is contained in:
@@ -1375,6 +1375,15 @@ def delete_profile(name: str, yes: bool = False) -> Path:
|
||||
if _closed:
|
||||
print(f"✓ Released {_closed} session database connection(s) held by this process")
|
||||
|
||||
# The Desktop serve process routes its agent/errors logs for every profile through one
|
||||
# QueueListener. On Windows those ConcurrentRotatingFileHandler instances retain their
|
||||
# ``.__*.lock`` files until explicitly closed, so rmtree otherwise fails with WinError 32.
|
||||
with contextlib.suppress(Exception):
|
||||
from hermes_logging import release_profile_log_handlers
|
||||
_released_logs = release_profile_log_handlers(profile_dir)
|
||||
if _released_logs:
|
||||
print(f"✓ Released {_released_logs} profile log handler(s) held by this process")
|
||||
|
||||
# 3. Remove wrapper script
|
||||
if has_wrapper and remove_wrapper_script(canon):
|
||||
print(f"✓ Removed {wrapper_path}")
|
||||
|
||||
@@ -475,6 +475,16 @@ class _ProfileRoutingFileHandler(logging.Handler):
|
||||
_quietly(handler.close)
|
||||
super().close()
|
||||
|
||||
def release_profile(self, home: Path) -> bool:
|
||||
"""Close and forget the routed files belonging to a deleted profile."""
|
||||
with self._profile_handlers_lock:
|
||||
handler = self._profile_handlers.pop(home, None)
|
||||
self._profile_homes.discard(home)
|
||||
if handler is None:
|
||||
return False
|
||||
_quietly(handler.close)
|
||||
return True
|
||||
|
||||
|
||||
# Asynchronous file logging: an ``emit`` can block on the cross-process
|
||||
# rotation lock (module header); on an asyncio thread that stalls the loop and
|
||||
@@ -580,6 +590,35 @@ def drain_log_queue(timeout: float = 1.0) -> None:
|
||||
t.join(timeout)
|
||||
|
||||
|
||||
def release_profile_log_handlers(profile_home: str | Path) -> int:
|
||||
"""Release this process's routed log files below a profile before it is removed.
|
||||
|
||||
The Desktop serve process can route records for several profiles through one
|
||||
``QueueListener``. On Windows, each routed concurrent log handler keeps its
|
||||
lock file open, so closing only external profile resources still leaves
|
||||
``logs/.__agent.lock`` and ``logs/.__errors.lock`` unavailable to rmtree.
|
||||
"""
|
||||
global _queue_listener
|
||||
try:
|
||||
home = Path(profile_home).expanduser().resolve()
|
||||
except (TypeError, ValueError, OSError):
|
||||
return 0
|
||||
|
||||
with _queue_state_lock:
|
||||
listener = _queue_listener
|
||||
if listener is not None:
|
||||
listener.stop()
|
||||
_queue_listener = None
|
||||
released = sum(
|
||||
handler.release_profile(home)
|
||||
for handler in _queued_file_handlers
|
||||
if isinstance(handler, _ProfileRoutingFileHandler)
|
||||
)
|
||||
if listener is not None:
|
||||
_start_queue_listener_locked()
|
||||
return released
|
||||
|
||||
|
||||
def enable_profile_log_routing(profile_homes: Sequence[str | Path]) -> bool:
|
||||
"""Make the queued file logs follow a desktop profile context.
|
||||
|
||||
|
||||
@@ -374,6 +374,25 @@ class TestBackfillProfileEnvs:
|
||||
class TestDeleteProfile:
|
||||
"""Tests for delete_profile()."""
|
||||
|
||||
def test_releases_routed_log_handlers_before_removing_profile(self, profile_env, monkeypatch):
|
||||
"""Deleting a Desktop profile releases this process's profile-scoped log handlers."""
|
||||
profile_dir = create_profile("coder", no_alias=True)
|
||||
released: list[Path] = []
|
||||
|
||||
monkeypatch.setattr(profiles, "_cleanup_gateway_service", lambda *_: None)
|
||||
monkeypatch.setattr(profiles, "_maybe_unregister_gateway_service", lambda *_: None)
|
||||
monkeypatch.setattr(profiles, "_stop_profile_backends", lambda *_: None)
|
||||
monkeypatch.setattr(profiles, "_notify_multiplexer", lambda *_: None)
|
||||
monkeypatch.setattr(
|
||||
"hermes_logging.release_profile_log_handlers",
|
||||
lambda home: released.append(home) or 2,
|
||||
)
|
||||
|
||||
delete_profile("coder", yes=True)
|
||||
|
||||
assert released == [profile_dir]
|
||||
assert not profile_dir.exists()
|
||||
|
||||
|
||||
def test_rmtree_failure_raises(self, profile_env):
|
||||
profile_dir = create_profile("coder", no_alias=True)
|
||||
|
||||
@@ -142,6 +142,48 @@ class TestSetupLogging:
|
||||
hermes_home / "logs" / "agent.log"
|
||||
).read_text()
|
||||
|
||||
def test_release_profile_log_handlers_closes_only_deleted_profile(self, hermes_home, tmp_path):
|
||||
"""Profile deletion releases its routed log files without disturbing another profile."""
|
||||
from hermes_constants import reset_hermes_home_override, set_hermes_home_override
|
||||
|
||||
deleted_home = tmp_path / "profile-deleted"
|
||||
other_home = tmp_path / "profile-other"
|
||||
deleted_home.mkdir()
|
||||
other_home.mkdir()
|
||||
hermes_logging.setup_logging(hermes_home=hermes_home)
|
||||
assert hermes_logging.enable_profile_log_routing(
|
||||
[hermes_home, deleted_home, other_home]
|
||||
) is True
|
||||
|
||||
logger = logging.getLogger("agent.profile-delete-log-release")
|
||||
token = set_hermes_home_override(deleted_home)
|
||||
try:
|
||||
logger.warning("deleted profile log handles")
|
||||
finally:
|
||||
reset_hermes_home_override(token)
|
||||
token = set_hermes_home_override(other_home)
|
||||
try:
|
||||
logger.warning("other profile log handles")
|
||||
finally:
|
||||
reset_hermes_home_override(token)
|
||||
hermes_logging.flush_log_queue()
|
||||
|
||||
routers = [
|
||||
handler for handler in hermes_logging._queued_file_handlers
|
||||
if isinstance(handler, hermes_logging._ProfileRoutingFileHandler)
|
||||
]
|
||||
assert len(routers) == 2 # agent.log and errors.log
|
||||
assert all(deleted_home.resolve() in handler._profile_handlers for handler in routers)
|
||||
assert all(other_home.resolve() in handler._profile_handlers for handler in routers)
|
||||
|
||||
assert hermes_logging.release_profile_log_handlers(deleted_home) == 2
|
||||
|
||||
assert all(deleted_home.resolve() not in handler._profile_handlers for handler in routers)
|
||||
assert all(deleted_home.resolve() not in handler._profile_homes for handler in routers)
|
||||
assert all(other_home.resolve() in handler._profile_handlers for handler in routers)
|
||||
assert "other profile log handles" in (other_home / "logs" / "agent.log").read_text()
|
||||
assert "other profile log handles" in (other_home / "logs" / "errors.log").read_text()
|
||||
|
||||
def test_a_second_home_routes_instead_of_stacking_an_unfiltered_handler(self, hermes_home, tmp_path):
|
||||
"""A dashboard or serve backend builds agents for several profiles in ONE process, and each
|
||||
one calls setup_logging for its own home. The second home must get a router — a bare file
|
||||
|
||||
Reference in New Issue
Block a user