From 40f9edbfe738c780a41bf713f7b3a6c5fe39fb85 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Tue, 29 Sep 2026 16:23:11 -0400 Subject: [PATCH] Admit viewer reuse inside atomic acquisition and retain external lifecycles --- src/zmqruntime/viewer_state.py | 34 +++++++++++++++++++-- tests/test_viewer_state.py | 54 ++++++++++++++++++++++++++++++++-- 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/src/zmqruntime/viewer_state.py b/src/zmqruntime/viewer_state.py index 5f66d0e..fa56a7f 100644 --- a/src/zmqruntime/viewer_state.py +++ b/src/zmqruntime/viewer_state.py @@ -13,6 +13,7 @@ import logging import threading import time +from abc import ABC, abstractmethod from dataclasses import dataclass, field from enum import Enum, auto from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Tuple @@ -61,6 +62,21 @@ def is_healthy(self) -> bool: VisualizerFactory = Callable[[], "VisualizerProcessManager"] +class ViewerReuseAdmissionABC(ABC): + """Caller-owned constraints admitted inside atomic viewer acquisition.""" + + @abstractmethod + def require_reusable(self, visualizer: "VisualizerProcessManager") -> None: + """Raise without changing the existing instance when reuse is forbidden.""" + + +class HealthyViewerReuseAdmission(ViewerReuseAdmissionABC): + """Generic callers require health but declare no application launch settings.""" + + def require_reusable(self, visualizer: "VisualizerProcessManager") -> None: + pass + + class ViewerStateManager: """ Singleton manager for viewer lifecycle state. @@ -123,6 +139,8 @@ def get_or_create_viewer( factory: VisualizerFactory, wait_for_ready: bool = True, ready_timeout: float = 30.0, + *, + reuse_admission: ViewerReuseAdmissionABC = HealthyViewerReuseAdmission(), ) -> Tuple["VisualizerProcessManager", bool]: """ Atomically get existing viewer or create a new one. @@ -136,6 +154,9 @@ def get_or_create_viewer( factory: Callable that creates a VisualizerProcessManager wait_for_ready: If True, block until viewer is ready (or timeout) ready_timeout: Maximum seconds to wait for ready state + reuse_admission: Application constraints checked under the acquisition + lock before returning a healthy existing instance. Rejection does + not stop, replace or forget the existing viewer. Returns: Tuple of (visualizer_instance, was_created) @@ -152,6 +173,7 @@ def get_or_create_viewer( if existing is not None: if existing.is_healthy: + reuse_admission.require_reusable(existing.visualizer) existing.last_used = time.time() logger.debug( "ViewerStateManager: Reusing existing %s viewer on port %d (state=%s)", @@ -215,7 +237,10 @@ def _start_and_wait( """Start the viewer process and optionally wait for ready state.""" try: # Start the process - instance.visualizer.start() + # A factory may attach to an already-live, externally-owned viewer. + # Registering that lifecycle must not restart its foreign process. + if not instance.visualizer.is_running: + instance.visualizer.start() logger.info( "ViewerStateManager: Started %s viewer on port %d", instance.viewer_type, @@ -482,6 +507,8 @@ def get_or_create_viewer( factory: VisualizerFactory, wait_for_ready: bool = True, ready_timeout: float = 30.0, + *, + reuse_admission: ViewerReuseAdmissionABC = HealthyViewerReuseAdmission(), ) -> Tuple["VisualizerProcessManager", bool]: """ Convenience function for atomic viewer get-or-create. @@ -494,4 +521,7 @@ def get_or_create_viewer( ) """ manager = ViewerStateManager.get_instance() - return manager.get_or_create_viewer(viewer_type, port, factory, wait_for_ready, ready_timeout) + return manager.get_or_create_viewer( + viewer_type, port, factory, wait_for_ready, ready_timeout, + reuse_admission=reuse_admission, + ) diff --git a/tests/test_viewer_state.py b/tests/test_viewer_state.py index 5125ee3..cbdb03b 100644 --- a/tests/test_viewer_state.py +++ b/tests/test_viewer_state.py @@ -1,6 +1,6 @@ import pytest -from zmqruntime.viewer_state import ViewerStateManager +from zmqruntime.viewer_state import ViewerReuseAdmissionABC, ViewerStateManager class RecordingVisualizer: @@ -9,11 +9,14 @@ def __init__(self, *, fail_start: bool = False) -> None: self.stop_calls = 0 self.force_stop_calls = 0 self.ready_timeouts = [] - self.running = True + self.running = False + self.start_calls = 0 def start(self): + self.start_calls += 1 if self.fail_start: raise RuntimeError("start failed") + self.running = True return None def wait_for_ready(self, timeout: float) -> bool: @@ -96,3 +99,50 @@ def test_viewer_state_subscription_owns_idempotent_release(viewer_manager): viewer_manager.release_viewer("napari", 5700) assert len(observed) == 2 + + +def test_reuse_admission_runs_atomically_and_preserves_rejected_viewer(viewer_manager): + import threading + + existing = RecordingVisualizer() + viewer_manager.get_or_create_viewer("napari", 5700, lambda: existing) + observations = [] + + class RejectReuse(ViewerReuseAdmissionABC): + def require_reusable(self, visualizer): + # A second thread cannot acquire the owner's lock during admission. + def probe(): + acquired = viewer_manager._lock.acquire(blocking=False) + observations.append(acquired) + if acquired: + viewer_manager._lock.release() + + thread = threading.Thread(target=probe) + thread.start() + thread.join(timeout=1) + assert not thread.is_alive() + assert visualizer is existing + raise ValueError("incompatible launch request") + + def forbidden_factory(): + raise AssertionError("rejected reuse must not construct or restart") + + with pytest.raises(ValueError, match="incompatible launch request"): + viewer_manager.get_or_create_viewer( + "napari", 5700, forbidden_factory, reuse_admission=RejectReuse() + ) + assert observations == [False] + assert viewer_manager.get_viewer("napari", 5700) is existing + assert existing.force_stop_calls == 0 + assert existing.start_calls == 1 + + +def test_live_external_factory_is_registered_without_start(viewer_manager): + existing = RecordingVisualizer(fail_start=True) + existing.running = True + acquired, created = viewer_manager.get_or_create_viewer( + "fiji", 5701, lambda: existing + ) + assert acquired is existing and created is True + assert viewer_manager.get_viewer("fiji", 5701) is existing + assert existing.start_calls == 0