From b4be726a3b86db67f3fb8a3ca1981b775aeb22fc Mon Sep 17 00:00:00 2001 From: Ahmed Allam Date: Sun, 4 Oct 2026 05:05:21 +0000 Subject: [PATCH] fix(runtime): terminate PTY exec streams on teardown and silence 3.14 response finalizer noise StrixDockerSandboxClient.delete() killed the container and then let the SDK's delete() run shutdown(), which only terminates the agent's PTY exec streams while the container still exists. Once the container was gone the hijacked exec sockets were left to the garbage collector and their HTTP responses failed to close at interpreter exit, which Python 3.14 reports as "Exception ignored while finalizing file " after the scan summary. delete() now awaits pty_terminate_all() first, whatever the container's state. The unraisable filter now also matches Python 3.14's finalizer shape (object=None, repr in err_msg) and http.client responses, and logs the match at DEBUG on strix.telemetry instead of letting it reach stderr. --- strix/runtime/docker_client.py | 12 ++++++- strix/telemetry/logging.py | 26 ++++++++++++-- tests/test_docker_client_delete.py | 50 ++++++++++++++++++++++++-- tests/test_unraisable_filter.py | 57 ++++++++++++++++++++++++++++-- 4 files changed, 136 insertions(+), 9 deletions(-) diff --git a/strix/runtime/docker_client.py b/strix/runtime/docker_client.py index 969267d5..979fdcb4 100644 --- a/strix/runtime/docker_client.py +++ b/strix/runtime/docker_client.py @@ -276,7 +276,17 @@ class StrixDockerSandboxClient(DockerSandboxClient): return session async def delete(self, session: SandboxSession) -> SandboxSession: - container_id = getattr(getattr(session._inner, "state", None), "container_id", None) + inner = session._inner + # The SDK's delete() only runs inner.shutdown() (which terminates the + # agent's PTY exec streams) when the container still exists. Terminate + # them unconditionally first: otherwise the hijacked exec sockets are + # left to the garbage collector and their HTTP responses fail to close + # at interpreter exit ("Exception ignored while finalizing file"). + pty_terminate_all = getattr(inner, "pty_terminate_all", None) + if pty_terminate_all is not None: + with contextlib.suppress(Exception): + await pty_terminate_all() + container_id = getattr(getattr(inner, "state", None), "container_id", None) if container_id: # Best-effort kill: NotFound/APIError cover a gone or unhappy # container. RequestException covers a torn-down daemon socket — diff --git a/strix/telemetry/logging.py b/strix/telemetry/logging.py index e3f8e592..3fcd4e5b 100644 --- a/strix/telemetry/logging.py +++ b/strix/telemetry/logging.py @@ -5,6 +5,7 @@ from __future__ import annotations import contextlib import logging import os +import re import sys import warnings from contextvars import ContextVar @@ -94,12 +95,25 @@ def configure_dependency_logging() -> None: _unraisable_hook_installed = False +_FINALIZER_NOISE_MODULES = frozenset({"urllib3", "http"}) +_FINALIZER_FILE_RE = re.compile(r"finalizing file <(urllib3|http\.client)\.") + + def _is_urllib3_closed_file_noise(unraisable: sys.UnraisableHookArgs) -> bool: - return ( + """An HTTP response (urllib3 or http.client) whose socket was closed first. + + Python 3.14 reports IOBase finalizer failures with ``object=None`` and the + object's repr inside ``err_msg`` ("Exception ignored while finalizing file + <...>"); earlier versions pass the object itself. + """ + if not ( isinstance(unraisable.exc_value, ValueError) and "I/O operation on closed file" in str(unraisable.exc_value) - and type(unraisable.object).__module__.split(".")[0] == "urllib3" - ) + ): + return False + if unraisable.object is not None: + return type(unraisable.object).__module__.split(".")[0] in _FINALIZER_NOISE_MODULES + return _FINALIZER_FILE_RE.search(unraisable.err_msg or "") is not None def _silence_urllib3_finalizer_noise() -> None: @@ -111,6 +125,12 @@ def _silence_urllib3_finalizer_noise() -> None: def hook(unraisable: sys.UnraisableHookArgs) -> None: if _is_urllib3_closed_file_noise(unraisable): + with contextlib.suppress(Exception): # the interpreter may be finalizing + logging.getLogger("strix.telemetry").debug( + "Ignored HTTP response finalizer error: %s: %s", + unraisable.err_msg or type(unraisable.object).__name__, + unraisable.exc_value, + ) return previous(unraisable) diff --git a/tests/test_docker_client_delete.py b/tests/test_docker_client_delete.py index ef93723c..241e86f7 100644 --- a/tests/test_docker_client_delete.py +++ b/tests/test_docker_client_delete.py @@ -36,9 +36,17 @@ def _client_with_kill_error(exc: Exception) -> StrixDockerSandboxClient: return client -def _session(container_id: str | None = "abc123") -> SandboxSession: - # delete() reads session._inner.state.container_id - fake = SimpleNamespace(_inner=SimpleNamespace(state=SimpleNamespace(container_id=container_id))) +def _session( + container_id: str | None = "abc123", pty_terminate_all: AsyncMock | None = None +) -> SandboxSession: + # delete() reads the inner state's container_id and awaits the inner + # session's PTY teardown. + fake = SimpleNamespace( + _inner=SimpleNamespace( + state=SimpleNamespace(container_id=container_id), + pty_terminate_all=pty_terminate_all or AsyncMock(), + ) + ) return cast("SandboxSession", fake) @@ -90,3 +98,39 @@ async def test_delete_noop_without_container_id() -> None: client.docker_client.containers.get.assert_not_called() super_delete.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_delete_terminates_pty_streams_even_when_the_container_is_gone() -> None: + """The SDK's delete() skips shutdown() (and with it PTY teardown) when the + container no longer exists, which leaves the agent's exec sockets to the + garbage collector. delete() must terminate them itself, before anything + else, whatever the container's state.""" + client = _client_with_kill_error(docker_errors.NotFound("gone")) + order: list[str] = [] + session = _session( + pty_terminate_all=AsyncMock(side_effect=lambda: order.append("pty_terminate_all")) + ) + + async def _super_delete(_self: object, _session: object) -> SandboxSession: + order.append("super.delete") + return session + + with patch.object(DockerSandboxClient, "delete", new=_super_delete): + await client.delete(session) + + assert order == ["pty_terminate_all", "super.delete"] + + +@pytest.mark.asyncio +async def test_delete_survives_pty_termination_errors() -> None: + client = StrixDockerSandboxClient.__new__(StrixDockerSandboxClient) + client.docker_client = MagicMock() + session = _session(pty_terminate_all=AsyncMock(side_effect=RuntimeError("daemon gone"))) + + with patch.object( + DockerSandboxClient, "delete", new=AsyncMock(return_value=session) + ) as super_delete: + await client.delete(session) + + super_delete.assert_awaited_once() diff --git a/tests/test_unraisable_filter.py b/tests/test_unraisable_filter.py index 83d267ba..33c305d0 100644 --- a/tests/test_unraisable_filter.py +++ b/tests/test_unraisable_filter.py @@ -1,3 +1,5 @@ +import http.client +import socket import sys import pytest @@ -8,11 +10,13 @@ from strix.telemetry.logging import _is_urllib3_closed_file_noise class _Args: - def __init__(self, exc_value: BaseException | None, obj: object) -> None: + def __init__( + self, exc_value: BaseException | None, obj: object, err_msg: str | None = None + ) -> None: self.exc_type = type(exc_value) if exc_value is not None else None self.exc_value = exc_value self.exc_traceback = None - self.err_msg = None + self.err_msg = err_msg self.object = obj @@ -25,6 +29,45 @@ def test_filters_urllib3_closed_file_noise() -> None: assert _is_urllib3_closed_file_noise(args) # type: ignore[arg-type] +def test_filters_http_client_closed_file_noise() -> None: + sock = socket.socket() + try: + response = http.client.HTTPResponse(sock) + finally: + sock.close() + args = _Args(ValueError("I/O operation on closed file."), response) + assert _is_urllib3_closed_file_noise(args) # type: ignore[arg-type] + + +@pytest.mark.parametrize( + "repr_text", + [ + "", + "", + ], +) +def test_filters_python314_finalizer_shape(repr_text: str) -> None: + """3.14 reports IOBase finalizer failures with object=None and the repr in err_msg.""" + args = _Args( + ValueError("I/O operation on closed file."), + None, + err_msg=f"Exception ignored while finalizing file {repr_text}", + ) + assert _is_urllib3_closed_file_noise(args) # type: ignore[arg-type] + + +def test_python314_shape_passes_through_other_files() -> None: + args = _Args( + ValueError("I/O operation on closed file."), + None, + err_msg="Exception ignored while finalizing file <_io.TextIOWrapper name='x' mode='w'>", + ) + assert not _is_urllib3_closed_file_noise(args) # type: ignore[arg-type] + assert not _is_urllib3_closed_file_noise( + _Args(ValueError("I/O operation on closed file."), None) # type: ignore[arg-type] + ) + + def test_passes_through_other_unraisables() -> None: assert not _is_urllib3_closed_file_noise( _Args(ValueError("I/O operation on closed file."), object()) # type: ignore[arg-type] @@ -46,6 +89,16 @@ def test_installed_hook_filters_and_delegates(monkeypatch: pytest.MonkeyPatch) - assert hook is not calls.append hook(_Args(ValueError("I/O operation on closed file."), _urllib3_response())) # type: ignore[arg-type] + hook( + _Args( # type: ignore[arg-type] + ValueError("I/O operation on closed file."), + None, + err_msg=( + "Exception ignored while finalizing file " + "" + ), + ) + ) assert calls == [] other = _Args(RuntimeError("boom"), object())