diff --git a/strix/telemetry/logging.py b/strix/telemetry/logging.py index 3fcd4e5b..3ebd91fd 100644 --- a/strix/telemetry/logging.py +++ b/strix/telemetry/logging.py @@ -95,7 +95,6 @@ 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)\.") @@ -112,7 +111,8 @@ def _is_urllib3_closed_file_noise(unraisable: sys.UnraisableHookArgs) -> bool: ): return False if unraisable.object is not None: - return type(unraisable.object).__module__.split(".")[0] in _FINALIZER_NOISE_MODULES + module = type(unraisable.object).__module__ + return module.split(".")[0] == "urllib3" or module == "http.client" return _FINALIZER_FILE_RE.search(unraisable.err_msg or "") is not None diff --git a/tests/test_docker_client_delete.py b/tests/test_docker_client_delete.py index 241e86f7..e950fc40 100644 --- a/tests/test_docker_client_delete.py +++ b/tests/test_docker_client_delete.py @@ -11,22 +11,28 @@ would let it escape and surface a traceback on every teardown. from __future__ import annotations +import uuid +from pathlib import Path from types import SimpleNamespace -from typing import TYPE_CHECKING, cast +from typing import cast from unittest.mock import AsyncMock, MagicMock, patch import pytest -from agents.sandbox.sandboxes.docker import DockerSandboxClient +from agents.sandbox.manifest import Manifest +from agents.sandbox.sandboxes.docker import ( + DockerSandboxClient, + DockerSandboxSession, + DockerSandboxSessionState, + _DockerExecSocket, + _DockerPtyProcessEntry, +) +from agents.sandbox.session.sandbox_session import SandboxSession from docker import errors as docker_errors from requests.exceptions import ConnectionError as RequestsConnectionError from strix.runtime.docker_client import StrixDockerSandboxClient -if TYPE_CHECKING: - from agents.sandbox.session.sandbox_session import SandboxSession - - def _client_with_kill_error(exc: Exception) -> StrixDockerSandboxClient: """A StrixDockerSandboxClient whose containers.get(...).kill() raises ``exc``.""" client = StrixDockerSandboxClient.__new__(StrixDockerSandboxClient) @@ -134,3 +140,65 @@ async def test_delete_survives_pty_termination_errors() -> None: await client.delete(session) super_delete.assert_awaited_once() + + +def _real_session_with_open_pty( + container_id: str = "abc123", +) -> tuple[SandboxSession, _DockerExecSocket]: + """A real SDK DockerSandboxSession holding one live PTY exec stream, the + shape exec_command leaves behind: a hijacked socket plus the streamed HTTP + response docker-py pins to it.""" + state = DockerSandboxSessionState.model_construct( + type="docker", + image="sandbox:test", + container_id=container_id, + manifest=Manifest(), + session_id=uuid.uuid4(), + exposed_ports=(), + workspace_root_ready=True, + ) + container = MagicMock() + container.client.api.exec_inspect.return_value = {"Running": False, "ExitCode": 0} + inner = DockerSandboxSession(docker_client=MagicMock(), container=container, state=state) + exec_socket = _DockerExecSocket(sock=MagicMock(), raw_sock=MagicMock(), response=MagicMock()) + inner._pty_processes[1] = _DockerPtyProcessEntry( + exec_id="exec-1", + sock=exec_socket, + raw_sock=exec_socket.raw_sock, + pid_path=Path("/workspace/.pty/1.pid"), + tty=True, + ) + return SandboxSession(inner), exec_socket + + +@pytest.mark.asyncio +async def test_sdk_delete_alone_leaves_pty_exec_streams_open_when_the_container_is_gone() -> None: + """Documents the SDK gap delete() compensates for: with the container gone, + DockerSandboxClient.delete() never terminates the PTY entries.""" + client = StrixDockerSandboxClient.__new__(StrixDockerSandboxClient) + client.docker_client = MagicMock() + client.docker_client.containers.get.side_effect = docker_errors.NotFound("gone") + session, exec_socket = _real_session_with_open_pty() + + await DockerSandboxClient.delete(client, session) + + assert session._inner._pty_processes # type: ignore[attr-defined] + cast("MagicMock", exec_socket.sock).close.assert_not_called() + cast("MagicMock", exec_socket.response).close.assert_not_called() + + +@pytest.mark.asyncio +async def test_delete_closes_real_pty_exec_streams_when_the_container_is_gone() -> None: + """End to end through the real SDK session and the real SDK delete(): the + exec socket and its pinned HTTP response are closed, so nothing is left for + the garbage collector at interpreter exit.""" + client = StrixDockerSandboxClient.__new__(StrixDockerSandboxClient) + client.docker_client = MagicMock() + client.docker_client.containers.get.side_effect = docker_errors.NotFound("gone") + session, exec_socket = _real_session_with_open_pty() + + await client.delete(session) + + assert not session._inner._pty_processes # type: ignore[attr-defined] + cast("MagicMock", exec_socket.sock).close.assert_called_once() + cast("MagicMock", exec_socket.response).close.assert_called_once() diff --git a/tests/test_unraisable_filter.py b/tests/test_unraisable_filter.py index 33c305d0..690629e6 100644 --- a/tests/test_unraisable_filter.py +++ b/tests/test_unraisable_filter.py @@ -1,4 +1,5 @@ import http.client +import http.cookiejar import socket import sys @@ -29,6 +30,11 @@ def test_filters_urllib3_closed_file_noise() -> None: assert _is_urllib3_closed_file_noise(args) # type: ignore[arg-type] +def test_ignores_closed_file_errors_from_other_http_modules() -> None: + args = _Args(ValueError("I/O operation on closed file."), http.cookiejar.CookieJar()) + assert not _is_urllib3_closed_file_noise(args) # type: ignore[arg-type] + + def test_filters_http_client_closed_file_noise() -> None: sock = socket.socket() try: