mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
09e72125b4
## Description On Apple Silicon (arm64) macOS the proxy hard-crashes with SIGSEGV the moment it compresses an image, and because the proxy is the single API endpoint for every routed client (`ANTHROPIC_BASE_URL=http://127.0.0.1:8787`), one image request takes down every agent on the machine at once — they then fail with `ConnectionRefused` and retry into a closed port until the proxy is manually restarted. The faulting stack is inside OpenCV's KleidiCV ARM NEON resize (`kleidicv::neon::kleidicv_resize_generic_stripe_u8`), reached from the SigLIP ONNX image encoder during `ImageCompressor.compress()`. The proxy runs that call on a `ThreadPoolExecutor` (`headroom/proxy/server.py:946`), and both handler call sites (`headroom/proxy/handlers/anthropic.py:1148-1172`, `headroom/proxy/handlers/openai.py:2274-2296`) wrap it in `try/except Exception` intending to fail open. That guard cannot help: a native SIGSEGV is not a Python exception, and a segfault on any worker thread aborts the whole interpreter. Thread isolation is not crash isolation. The defect Headroom owns is that an optional, best-effort, native-heavy transform runs in-process with no crash boundary, so any native fault in it is fatal to the proxy and to every unrelated client it fronts. This change gives image compression a real crash boundary by executing it in a spawned subprocess, so a native crash degrades to "image forwarded uncompressed" instead of killing the proxy. Closes #2107. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - Added `headroom/proxy/image_isolation.py` with `run_image_compression_isolated(messages, provider, *, timeout) -> tuple[list[dict], dict | None]`, which runs an `ImageCompressor.compress` worker inside a lazily-created module-level `ProcessPoolExecutor(max_workers=1)` on a **spawn** multiprocessing context (ONNX sessions are not fork-safe) and carries the compression result (technique, `savings_percent`, token counts) back across the process boundary. The native OpenCV/KleidiCV work now runs in the child address space. - Made the runner fail open for **any** child outcome: `BrokenProcessPool` (the class raised when the child is killed by a signal, i.e. SIGSEGV/SIGABRT), `TimeoutError`, or any other `Exception` all return `(messages, None)` — the original `messages` unchanged, no telemetry — and reset the pool so the next request re-spawns a fresh child. - Routed the two request-path image-compression sites (`handlers/anthropic.py`, `handlers/openai.py`) through the runner, keeping the existing `ImageCompressionDecision` gate and the `image_compression` mutation tag, and emitting the savings `INFO` log line from the runner's returned result on the success path. - Scoped strictly to crash containment: `config.image_optimize` default is unchanged, no dependency pins, no new env switches. ## Testing - [x] Unit tests pass (`uv run pytest tests/test_image_compression_isolation.py tests/test_image_compression_offload.py`) - [x] Linting passes (`uv run ruff check .`) - [ ] Type checking passes (`uv run mypy headroom`) - [x] New tests added for new functionality when applicable - [x] Manual testing performed ### Test Output ```text $ uv run pytest tests/test_image_compression_isolation.py tests/test_image_compression_offload.py -q ....... [100%] 7 passed in 1.73s ``` The reproduction test (`test_worker_sigsegv_fails_open_parent_survives`) spawns a real subprocess whose worker dies by signal (`os.abort()`), then asserts `run_image_compression_isolated` returns the original messages and that the parent test process is still alive and continues past the call. ## Real Behavior Proof - Environment: Windows 11, Python 3.12, `uv run`; no live provider. - Exact command / steps: `uv run pytest tests/test_image_compression_isolation.py tests/test_image_compression_offload.py -q`, which drives the runner against a real spawned subprocess that is killed by signal, one that raises, one that times out, and one that returns normally, and also locks the handler wiring to `run_image_compression_isolated(...)`. - Observed result: on a signal-killed child the runner returns the original message list and the parent survives; on a raising or timing-out child it fails open the same way; on a normal child the compressed messages are returned. The handler source-level regression keeps the savings log and mutation-tag path wired through the new isolation helper. Before the change, the handlers called `compressor.compress(...)` through `_run_compression_in_executor(...)`, so a native crash on that worker thread would abort the interpreter. - Not tested: live arm64 macOS run against a real `opencv-python` 5.x KleidiCV fault. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes - Non-goals, kept out deliberately to keep this slice shippable now: pinning `opencv-python<5` (a transitive-dependency change that only masks this one fault and does not contain the next native crash), a `HEADROOM_IMAGE_OPTIMIZE=0` env off-switch (distinct config surface; `config.image_optimize=False` already disables the feature), the per-request ONNX model reload, and the negative-savings (`preserve` logged as `-100%`) reporting bug. The last two are independent defects noted in the same report and are better fixed on their own. - `mypy` left unchecked: not part of the focused validation for this change. --------- Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
117 lines
3.5 KiB
Python
117 lines
3.5 KiB
Python
from __future__ import annotations
|
|
|
|
import inspect
|
|
|
|
import pytest
|
|
|
|
from headroom.proxy import image_isolation
|
|
from headroom.proxy.handlers.anthropic import AnthropicHandlerMixin
|
|
from headroom.proxy.handlers.openai import OpenAIHandlerMixin
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _reset_image_pool() -> None:
|
|
image_isolation._reset_image_pool()
|
|
try:
|
|
yield
|
|
finally:
|
|
image_isolation._reset_image_pool()
|
|
image_isolation._IMAGE_WORKER = image_isolation._compress_messages_worker
|
|
|
|
|
|
def _messages() -> list[dict[str, object]]:
|
|
return [{"role": "user", "content": "image payload"}]
|
|
|
|
|
|
async def test_worker_sigsegv_fails_open_parent_survives(monkeypatch) -> None:
|
|
messages = _messages()
|
|
monkeypatch.setattr(image_isolation, "_IMAGE_WORKER", image_isolation._abort_worker)
|
|
|
|
returned, result = await image_isolation.run_image_compression_isolated(
|
|
messages,
|
|
provider="openai",
|
|
timeout=5.0,
|
|
)
|
|
|
|
assert returned is messages
|
|
assert result is None
|
|
|
|
monkeypatch.setattr(image_isolation, "_IMAGE_WORKER", image_isolation._success_worker)
|
|
recovered, recovered_result = await image_isolation.run_image_compression_isolated(
|
|
messages,
|
|
provider="openai",
|
|
timeout=5.0,
|
|
)
|
|
|
|
assert recovered[-1]["content"] == "compressed:openai"
|
|
assert recovered_result is not None
|
|
|
|
|
|
async def test_worker_exception_fails_open(monkeypatch) -> None:
|
|
messages = _messages()
|
|
monkeypatch.setattr(image_isolation, "_IMAGE_WORKER", image_isolation._raise_worker)
|
|
|
|
returned, result = await image_isolation.run_image_compression_isolated(
|
|
messages,
|
|
provider="anthropic",
|
|
timeout=5.0,
|
|
)
|
|
|
|
assert returned is messages
|
|
assert result is None
|
|
|
|
|
|
async def test_worker_timeout_fails_open(monkeypatch) -> None:
|
|
messages = _messages()
|
|
monkeypatch.setattr(image_isolation, "_IMAGE_WORKER", image_isolation._sleep_worker)
|
|
|
|
returned, result = await image_isolation.run_image_compression_isolated(
|
|
messages,
|
|
provider="openai",
|
|
timeout=0.05,
|
|
)
|
|
|
|
assert returned is messages
|
|
assert result is None
|
|
|
|
monkeypatch.setattr(image_isolation, "_IMAGE_WORKER", image_isolation._success_worker)
|
|
recovered, recovered_result = await image_isolation.run_image_compression_isolated(
|
|
messages,
|
|
provider="openai",
|
|
timeout=5.0,
|
|
)
|
|
|
|
assert recovered[-1]["content"] == "compressed:openai"
|
|
assert recovered_result is not None
|
|
|
|
|
|
async def test_worker_success_returns_compressed(monkeypatch) -> None:
|
|
messages = _messages()
|
|
monkeypatch.setattr(image_isolation, "_IMAGE_WORKER", image_isolation._success_worker)
|
|
|
|
returned, result = await image_isolation.run_image_compression_isolated(
|
|
messages,
|
|
provider="anthropic",
|
|
timeout=5.0,
|
|
)
|
|
|
|
assert returned[-1]["content"] == "compressed:anthropic"
|
|
assert result == {
|
|
"technique": "preserve",
|
|
"original_tokens": 100,
|
|
"compressed_tokens": 60,
|
|
"confidence": 1.0,
|
|
"savings_percent": 40.0,
|
|
}
|
|
|
|
|
|
def test_success_marks_mutation_and_logs() -> None:
|
|
anthropic_src = inspect.getsource(AnthropicHandlerMixin.handle_anthropic_messages)
|
|
openai_src = inspect.getsource(OpenAIHandlerMixin.handle_openai_chat)
|
|
|
|
assert "run_image_compression_isolated(" in anthropic_src
|
|
assert 'body_mutation_tracker.mark_mutated("image_compression")' in anthropic_src
|
|
assert "Image compression:" in anthropic_src
|
|
|
|
assert "run_image_compression_isolated(" in openai_src
|
|
assert "Image:" in openai_src
|