mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: Fix hyperlight WasmSandbox cross-thread Drop and harden hosted-agent sample (#5603)
* update hyperlight to beta and move samples, add hosted agent sample * Python: Fix hyperlight WasmSandbox cross-thread Drop and harden sample Root cause: when a worker-side closure raised, the exception's __traceback__ retained frame locals that included the partially constructed PyO3 sandbox. Future.result() re-raised that exception on the caller thread, and when the caller's exception was eventually GC'd the frame locals were released off-thread, dec_ref'ing the unsendable sandbox from the wrong thread and tripping the PyO3 panic '_native_wasm::WasmSandbox is unsendable, but is being dropped on another thread'. Fix: * Add _SandboxWorker._run_on_worker which catches every exception on the worker, drops __traceback__ there, deletes the original exception, and re-raises a fresh instance on the caller thread. initialize and execute route through it; dispose keeps its bare-submit semantics. * Add an opt-in diagnostic module _drop_diagnostic (no-op unless HYPERLIGHT_TRACE_DROPS=1) that installs a sys.unraisablehook and dumps owner-thread + per-thread stacks on any future cross-thread unsendable Drop. Useful for triaging similar PyO3 regressions. * Tests: cross-thread invocation, traceback-leak isolation, _SandboxEntry attribute-shape check, and a stale-reference stress test driven through asyncio.to_thread. Sample (samples/04-hosting/foundry-hosted-agents/responses/06_hyperlight_codeact): * Dockerfile installs agent-framework-* from in-tree source with python/ as build context so unreleased fixes can be validated end-to-end. * call_server.py pins the Responses API version. * main.py enables include_detailed_errors=True so future tool failures surface the actual exception text instead of a bare 'Error: Function failed.' string. * README.md documents the in-tree-package build and the Hyperlight hypervisor requirement (/dev/kvm on Linux, MSHV on Windows). Hosted environments without hypervisor passthrough surface 'No Hypervisor was found for Sandbox'; this is a hosting constraint, not a hyperlight bug. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Python: remove _drop_diagnostic from hyperlight package The diagnostic module was useful while bisecting the cross-thread Drop bug, but it is no longer needed now that _SandboxWorker._run_on_worker prevents the panic at the source. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Python: address PR review feedback on hyperlight - Use lazy agent_framework.hyperlight import in sample main.py. - Env-driven endpoint (FOUNDRY_AGENT_ENDPOINT) in call_server.py; remove personal URLs. - Align agent.yaml model deployment with manifest (gpt-4.1-mini). - Tighten Dockerfile requirements guard; drop dangling deploy.ps1 reference. - Preserve exception args when sanitizing tracebacks in _run_on_worker. - Add public _SandboxWorker.is_alive(); update test to avoid private attr. - Add namespace coverage tests for agent_framework.hyperlight lazy loader. - Add prominent note: Foundry hosted-agent runtime does not yet support Hyperlight (no hypervisor exposed); container works locally with /dev/kvm. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Python: bump hyperlight-sandbox dependencies to 0.4.x Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Python: renumber hyperlight codeact sample to 08 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Coerce worker exception args to strings for cross-thread safety Stringify exc.args on the worker thread before propagating, so any PyO3 unsendable object captured in args (e.g. via a caller-supplied callback or underlying SDK) cannot be Dropped on the calling thread. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * moved sample --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Eduard van Valkenburg
committed by
GitHub
Unverified
GitHub
parent
36b9b41e3b
commit
57c901a245
@@ -3,6 +3,9 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import contextlib
|
||||
import dataclasses
|
||||
import gc
|
||||
import importlib.metadata
|
||||
import importlib.util
|
||||
import inspect
|
||||
@@ -1042,9 +1045,8 @@ def test_sandbox_registry_close_shuts_down_workers(monkeypatch: pytest.MonkeyPat
|
||||
registry.close()
|
||||
|
||||
assert registry._entries == {}
|
||||
# Submitting after shutdown must fail; this proves the executor was actually torn down.
|
||||
with pytest.raises(RuntimeError):
|
||||
worker.submit(lambda: None)
|
||||
# After shutdown, the worker must report itself as no longer accepting work.
|
||||
assert worker.is_alive() is False
|
||||
|
||||
|
||||
def test_sandbox_registry_close_releases_per_entry_resources(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
|
||||
@@ -1125,3 +1127,243 @@ async def test_make_sandbox_callback_propagates_exceptions() -> None:
|
||||
callback = execute_code_module._make_sandbox_callback(boom)
|
||||
with pytest.raises(RuntimeError, match="nope"):
|
||||
callback(x=1)
|
||||
|
||||
|
||||
class _OwnerThreadTrackedResult:
|
||||
"""Fake sandbox.run() return value that mirrors a PyO3 ``unsendable`` object's Drop.
|
||||
|
||||
Records (rather than panics, since CPython swallows __del__ exceptions) the OS thread
|
||||
that finalized the object, so tests can assert it was dropped on the sandbox's owner
|
||||
thread and not on whatever thread happened to GC it.
|
||||
"""
|
||||
|
||||
drop_thread_violations: list[str] = []
|
||||
|
||||
def __init__(self, *, owner_thread: int, success: bool = True, stdout: str = "", stderr: str = "") -> None:
|
||||
self._owner_thread = owner_thread
|
||||
self.success = success
|
||||
self.stdout = stdout
|
||||
self.stderr = stderr
|
||||
|
||||
def __del__(self) -> None:
|
||||
ident = threading.get_ident()
|
||||
if ident != self._owner_thread:
|
||||
type(self).drop_thread_violations.append(
|
||||
f"_OwnerThreadTrackedResult dropped on thread {ident}, owner was {self._owner_thread}"
|
||||
)
|
||||
|
||||
|
||||
class _ResultDropTrackingFakeSandbox(_FakeSandbox):
|
||||
"""Fake sandbox whose ``run()`` returns an owner-thread-tracking result."""
|
||||
|
||||
def __init__(self, **kwargs: Any) -> None:
|
||||
super().__init__(**kwargs)
|
||||
self._owner_thread = threading.get_ident()
|
||||
|
||||
def run(self, code: str) -> Any:
|
||||
del code
|
||||
# Real Hyperlight runs almost always have non-empty stdout (the executed Python
|
||||
# ``print`` output); that is the path where _build_execution_contents attaches
|
||||
# raw_representation=result and the unsendable object escapes the worker thread.
|
||||
return _OwnerThreadTrackedResult(owner_thread=self._owner_thread, success=True, stdout="hello\n")
|
||||
|
||||
|
||||
def test_sandbox_run_result_is_finalized_on_owner_thread(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Regression: the object returned by ``sandbox.run`` must not escape its owner thread.
|
||||
|
||||
The Hyperlight ``WasmSandbox`` is unsendable; the value its ``run()`` returns can carry
|
||||
a back-reference to the sandbox and is itself unsendable. Attaching it to
|
||||
``Content.raw_representation`` lets it ride out of the worker thread and be garbage
|
||||
collected on whichever thread the asyncio loop / agent state ends up on, which trips
|
||||
the PyO3 ``Drop`` panic. Drop must happen on the worker thread that ran ``run()``.
|
||||
"""
|
||||
_OwnerThreadTrackedResult.drop_thread_violations.clear()
|
||||
_FakeSandbox.instances.clear()
|
||||
monkeypatch.setattr(execute_code_module, "_load_sandbox_class", lambda: _ResultDropTrackingFakeSandbox)
|
||||
|
||||
execute_code = HyperlightExecuteCodeTool()
|
||||
|
||||
def _drive() -> None:
|
||||
# Run the whole invocation inside a helper frame so every local
|
||||
# reference (contents, awaitable, asyncio frames) dies when the
|
||||
# function returns. Anything still pinning the result is the bug.
|
||||
contents = asyncio.run(execute_code.invoke(arguments={"code": "None"}))
|
||||
assert contents and contents[0].type == "text"
|
||||
|
||||
_drive()
|
||||
for _ in range(3):
|
||||
gc.collect()
|
||||
|
||||
assert _OwnerThreadTrackedResult.drop_thread_violations == []
|
||||
|
||||
|
||||
def test_sandbox_is_finalized_on_owner_thread_after_registry_close(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Regression: dropping the sandbox object itself must occur on its owner thread.
|
||||
|
||||
``_SandboxRegistry.close()`` previously held entries in a local list whose lifetime
|
||||
extended onto the caller's thread. When that list went out of scope the unsendable
|
||||
sandbox was finalized on the caller's thread, panicking PyO3 with
|
||||
"WasmSandbox is unsendable, but is being dropped by another thread".
|
||||
"""
|
||||
drop_violations: list[str] = []
|
||||
|
||||
class _OwnerDropFakeSandbox(_FakeSandbox):
|
||||
def __init__(self, **kwargs: Any) -> None:
|
||||
super().__init__(**kwargs)
|
||||
self._owner_thread = threading.get_ident()
|
||||
# Do not pin ourselves on the class-level instances list; we want the
|
||||
# registry/entry to hold the only strong reference so that dispose-time
|
||||
# drop is what determines the finalizer thread.
|
||||
_FakeSandbox.instances.remove(self)
|
||||
|
||||
def __del__(self) -> None:
|
||||
ident = threading.get_ident()
|
||||
if ident != self._owner_thread:
|
||||
drop_violations.append(f"sandbox dropped on thread {ident}, owner was {self._owner_thread}")
|
||||
|
||||
monkeypatch.setattr(execute_code_module, "_load_sandbox_class", lambda: _OwnerDropFakeSandbox)
|
||||
|
||||
registry = execute_code_module._SandboxRegistry()
|
||||
execute_code = HyperlightExecuteCodeTool(_registry=registry)
|
||||
asyncio.run(execute_code.invoke(arguments={"code": "None"}))
|
||||
|
||||
registry.close()
|
||||
|
||||
# Release the registry/tool references and force a GC. With the fix in place the
|
||||
# sandbox is already disposed on the worker thread inside close(); dropping these
|
||||
# local references must not trigger a wrong-thread __del__ now.
|
||||
del registry
|
||||
del execute_code
|
||||
for _ in range(3):
|
||||
gc.collect()
|
||||
|
||||
assert drop_violations == [], f"sandbox was dropped off-thread despite registry close: {drop_violations}"
|
||||
|
||||
|
||||
def test_worker_failure_does_not_leak_unsendable_via_exception_traceback(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Regression: an exception raised inside a worker closure must not leak unsendable refs.
|
||||
|
||||
Production failure mode: ``_build_sandbox`` (or ``sandbox.run``) raises on the
|
||||
worker thread. ``concurrent.futures`` propagates the exception via
|
||||
``Future.result()`` to the caller's thread. Python's exception object retains
|
||||
``__traceback__`` whose frames reference local variables -- including the
|
||||
partially-built PyO3 unsendable sandbox. When the caller's thread eventually
|
||||
GCs the exception, those locals are dec_ref'd on the wrong thread and PyO3
|
||||
panics with
|
||||
``_native_wasm::WasmSandbox is unsendable, but is being dropped on another thread``.
|
||||
|
||||
The fix routes every worker closure through ``_run_on_worker``, which catches
|
||||
the exception on the worker thread, drops its traceback there, and re-raises
|
||||
a fresh exception on the caller side carrying only the message.
|
||||
"""
|
||||
drop_violations: list[str] = []
|
||||
|
||||
class _RaisingFakeSandbox(_FakeSandbox):
|
||||
def __init__(self, **kwargs: Any) -> None:
|
||||
super().__init__(**kwargs)
|
||||
self._owner_thread = threading.get_ident()
|
||||
_FakeSandbox.instances.remove(self)
|
||||
# Simulate production bug: build raises while ``self`` is alive in
|
||||
# the calling frame's locals -- the exception traceback will retain
|
||||
# a reference to this object.
|
||||
raise RuntimeError("simulated build failure with unsendable in frame locals")
|
||||
|
||||
def __del__(self) -> None:
|
||||
ident = threading.get_ident()
|
||||
if ident != self._owner_thread:
|
||||
drop_violations.append(f"sandbox dropped on thread {ident}, owner was {self._owner_thread}")
|
||||
|
||||
monkeypatch.setattr(execute_code_module, "_load_sandbox_class", lambda: _RaisingFakeSandbox)
|
||||
|
||||
registry = execute_code_module._SandboxRegistry()
|
||||
execute_code = HyperlightExecuteCodeTool(_registry=registry)
|
||||
|
||||
async def _drive(tool: HyperlightExecuteCodeTool) -> None:
|
||||
for _ in range(4):
|
||||
with contextlib.suppress(Exception):
|
||||
await tool.invoke(arguments={"code": "None"})
|
||||
|
||||
asyncio.run(_drive(execute_code))
|
||||
registry.close()
|
||||
|
||||
del registry
|
||||
del execute_code
|
||||
for _ in range(5):
|
||||
gc.collect()
|
||||
|
||||
assert drop_violations == [], (
|
||||
f"sandbox dropped off-thread despite worker raising on the owner thread: {drop_violations}"
|
||||
)
|
||||
|
||||
|
||||
def test_sandbox_entry_does_not_expose_unsendable_attributes() -> None:
|
||||
"""Architectural regression: the entry must not hold sandbox/snapshot as attributes.
|
||||
|
||||
The unsendable PyO3 sandbox/snapshot must live ONLY inside the per-entry worker
|
||||
thread, accessible only via worker-submitted closures. Any direct ``entry.sandbox``
|
||||
or ``entry.snapshot`` attribute would let callers obtain a strong reference that
|
||||
can be released on a non-owner thread, triggering PyO3's unsendable Drop panic
|
||||
(the production bug we are fixing).
|
||||
"""
|
||||
fields = {f.name for f in dataclasses.fields(execute_code_module._SandboxEntry)}
|
||||
assert "sandbox" not in fields, "_SandboxEntry must not expose `sandbox` directly"
|
||||
assert "snapshot" not in fields, "_SandboxEntry must not expose `snapshot` directly"
|
||||
# Whatever attributes remain must be sendable / safe to GC on any thread.
|
||||
assert fields <= {"worker", "input_dir", "output_dir"}
|
||||
|
||||
|
||||
def test_sandbox_survives_external_thread_holding_stale_reference(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Regression: stale refs held by external executors must not cause wrong-thread Drop.
|
||||
|
||||
Production traceback was ``concurrent.futures.thread._worker:95 del work_item`` on
|
||||
``asyncio_0`` -- an external ``ThreadPoolExecutor`` whose ``_WorkItem`` transitively
|
||||
held a strong reference to the sandbox via ``self._registry.execute``. When that
|
||||
work_item was deleted on the external worker thread, the sandbox's refcount could
|
||||
reach zero there, panicking PyO3.
|
||||
|
||||
With the actor-model refactor, ``HyperlightExecuteCodeTool._run_code`` runs the
|
||||
sandbox call via ``asyncio.to_thread(self._registry.execute, ...)`` which creates
|
||||
an external work_item containing ``self._registry.execute`` -- but that reference
|
||||
transitively holds only the registry, not the sandbox. The sandbox lives entirely
|
||||
inside the per-entry ``_SandboxWorker`` and never escapes; so when the external
|
||||
work_item is deleted on a non-owner thread, the sandbox's refcount cannot reach
|
||||
zero there.
|
||||
"""
|
||||
drop_violations: list[str] = []
|
||||
|
||||
class _OwnerDropFakeSandbox(_FakeSandbox):
|
||||
def __init__(self, **kwargs: Any) -> None:
|
||||
super().__init__(**kwargs)
|
||||
self._owner_thread = threading.get_ident()
|
||||
_FakeSandbox.instances.remove(self)
|
||||
|
||||
def __del__(self) -> None:
|
||||
ident = threading.get_ident()
|
||||
if ident != self._owner_thread:
|
||||
drop_violations.append(f"sandbox dropped on thread {ident}, owner was {self._owner_thread}")
|
||||
|
||||
monkeypatch.setattr(execute_code_module, "_load_sandbox_class", lambda: _OwnerDropFakeSandbox)
|
||||
|
||||
registry = execute_code_module._SandboxRegistry()
|
||||
execute_code = HyperlightExecuteCodeTool(_registry=registry)
|
||||
|
||||
async def _drive_many(tool: HyperlightExecuteCodeTool) -> None:
|
||||
# Many concurrent invocations push work_items into asyncio's default executor;
|
||||
# each work_item's args transitively reference the registry. If the registry
|
||||
# were the sandbox holder, the work_items' deletion on asyncio_0/asyncio_1 etc.
|
||||
# could trigger a wrong-thread Drop -- which is exactly the production bug.
|
||||
await asyncio.gather(*[tool.invoke(arguments={"code": "None"}) for _ in range(8)])
|
||||
|
||||
asyncio.run(_drive_many(execute_code))
|
||||
registry.close()
|
||||
|
||||
del registry
|
||||
del execute_code
|
||||
for _ in range(5):
|
||||
gc.collect()
|
||||
|
||||
assert drop_violations == []
|
||||
|
||||
Reference in New Issue
Block a user