Commit graph

1 commit

Author SHA1 Message Date
Manmit Singh
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.
2026-07-02 16:00:55 -07:00