mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
1 commit
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
bec47a1898
|
fix(memory): singleflight LocalBackend init to stop cold-start races (#1691)
## Description Running the proxy with `--memory` against a large context throws a bare `AssertionError` (empty message, ~0.1s elapsed, no upstream call) on every request; dropping `--memory` makes it go away. Per-project backends handed out by `BackendRouter._get_or_create_backend` init lazily on first use. `LocalBackend._ensure_initialized` guarded init with a bare `if not self._initialized:` and no `asyncio.Lock`, so concurrent first callers each kicked off a parallel `HierarchicalMemory.create()`. A slow cold-start (>2s on the `pytorch_mps` embedder) cancelled by the outer 2s memory-context `wait_for` left the backend half-built (`_hierarchical_memory` still `None`), so the retry tripped `assert self._hierarchical_memory is not None` (local.py:237/385/...) — the empty-message crash. `MemoryHandler._ensure_initialized` already uses a double-checked `asyncio.Lock`; the per-project `LocalBackend` never got the same treatment. Closes #1678 ## 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 - Add a lazily-created `asyncio.Lock` singleflight with a double-checked flag to `LocalBackend._ensure_initialized`, mirroring the existing `MemoryHandler` pattern — concurrent first callers await one init instead of racing N. - On `CancelledError` (e.g. the outer `wait_for` timeout mid cold-start), reset `_hierarchical_memory`/`_graph`/`_initialized` and re-raise, so a cancelled init never leaves a half-built backend for the next request to assert on. - Move the init body verbatim into `_init_locked()` (called with the lock held); the large diff is the dedent, no logic change. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ pytest tests/test_local_backend_init_race.py tests/test_memory_handler_concurrent_init.py -q tests/test_local_backend_init_race.py .. [ 20%] tests/test_memory_handler_concurrent_init.py .....s.. [100%] 9 passed, 1 skipped in 0.50s $ ruff check headroom/memory/backends/local.py tests/test_local_backend_init_race.py All checks passed! $ ruff format --check headroom/memory/backends/local.py tests/test_local_backend_init_race.py 2 files already formatted $ mypy headroom/memory/backends/local.py Success: no issues found in 1 source file ``` ## Real Behavior Proof - Environment: macOS (arm64), Python 3.14, local `.venv`. - Exact command / steps: `pytest tests/test_local_backend_init_race.py tests/test_memory_handler_concurrent_init.py -q`. First test spawns 10 concurrent first callers against a `LocalBackend` with a patched slow `HierarchicalMemory.create` and asserts `create` runs exactly once; second cancels a cold-start via an outer `asyncio.wait_for` timeout, asserts state resets to `None`/uninitialized, then a later call re-inits cleanly. - Observed result: both pass; `create` is called once under contention, and a cancelled init leaves no half-built backend. - Not tested: no end-to-end repro of the original `--memory` crash against a real large context / GPU embedder — the race is reproduced deterministically at the unit level instead. ## 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 - [ ] 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 - [ ] I have updated the CHANGELOG.md if applicable ## Additional Notes Docs/CHANGELOG unchanged — internal concurrency fix with no user-facing API or behavior change beyond removing the crash. |