mirror of
https://github.com/usestrix/strix.git
synced 2026-10-05 02:41:38 +00:00
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 <urllib3.response.HTTPResponse>" 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.
This commit is contained in:
parent
25f3978804
commit
b4be726a3b
4 changed files with 136 additions and 9 deletions
|
|
@ -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 —
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
[
|
||||
"<urllib3.response.HTTPResponse object at 0x7f2d3c1f0b10>",
|
||||
"<http.client.HTTPResponse object at 0x7f2d3c1f0b10>",
|
||||
],
|
||||
)
|
||||
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 "
|
||||
"<urllib3.response.HTTPResponse object at 0x1>"
|
||||
),
|
||||
)
|
||||
)
|
||||
assert calls == []
|
||||
|
||||
other = _Args(RuntimeError("boom"), object())
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue