# Memory Leak Remediation Plan — Remaining Fixes

Applied fixes (this commit):
- **Fix 1**: SQLite connection leak — `get_connection()` now a contextmanager that closes on exit (`storage.py`)
- **Fix 2**: WS handler no longer blocks on balance snapshot — deferred to single-worker ThreadPoolExecutor (`services_orders.py`)

---

## Fix 3 — WS reconnect backoff (`bitstamp_private_ws.py`)

**Problem**: `private_ws_main` retries with a fixed 5-second delay after any exception. If Bitstamp drops the connection repeatedly (rate limit, network blip), this creates a tight reconnect loop. Each reconnect calls `list_accounts` (opens a DB connection) and `_get_token` (HTTP call to Bitstamp), amplifying load.

**Fix**: Replace the fixed `WS_RECONNECT_SECONDS` sleep with exponential backoff:
- Start at 5s, double on each consecutive failure, cap at 60s.
- Reset to 5s after a successful connection that lasts > 30 seconds.
- Log the backoff delay so tight loops are visible in logs.

**Location**: `private_ws_main` and `_run_once` in `bitstamp_private_ws.py`.

---

## Fix 4 — `_CACHE_LOCKS` unbounded growth (`services_bitstamp.py`)

**Problem**: `_CACHE_LOCKS` dict grows every time a new `cache_key` is used (per account, per market symbol, etc.). Locks are never removed, even when the account is deleted or the cache entry expires.

**Fix options** (pick one):
- **A**: Use `weakref.WeakValueDictionary` — locks get GC'd when no caller holds a reference. Simplest, but the `with _cache_lock(cache_key):` pattern keeps a strong reference during use, so the lock survives the call and is collected after.
- **B**: Add a `_remove_cache_lock(cache_key)` call in `invalidate_account_cache` and account deletion paths. More explicit but requires wiring up cleanup at every deletion site.

**Location**: `_CACHE_LOCKS`, `_cache_lock` in `services_bitstamp.py:24-46`.

---

## Fix 5 — `api_cache` table eviction (`repo.py` / `server.py`)

**Problem**: `cache_put` does an upsert, so existing keys don't duplicate. But expired rows are never deleted — `cache_get` skips them, but they stay in the SQLite file. Over weeks, the table grows and the file bloats. Same applies to `order_records` and `account_balance_snapshots` (no retention policy).

**Fix**: Add a `_evict_expired_cache()` function in `repo.py`:
```python
def evict_expired_cache() -> int:
    now = utc_now_iso()
    with get_connection() as conn:
        count = conn.execute("DELETE FROM api_cache WHERE expires_at <= ?", (now,)).rowcount
        conn.commit()
    return count
```
Call it from the existing metadata refresh loop (`_metadata_refresh_loop` in `server.py`) every cycle — it already runs every 24 hours, which is sufficient for cache eviction.

For `order_records` and `account_balance_snapshots`, consider a configurable retention (e.g., keep last 90 days, or last 10,000 rows per account). This is a bigger change and should be a separate discussion.

**Location**: `repo.py`, `_metadata_refresh_loop` in `server.py`.

---

## Fix 6 — Price lookup caching in `crypto_client.py`

**Problem**: `_price_for_asset` calls `get_crypto_price` which calls `get_price_sync`, which creates a `ThreadPoolExecutor` + `asyncio.run()` + MCP SSE connection **per asset, per call**. A balance with 10 assets triggers 10 separate SSE connections. Now that this runs in the background (Fix 2), it won't block the WS loop, but it's still very wasteful.

**Fix**: Add a short-TTL in-memory price cache (e.g., 5 seconds) in `crypto_client.py`:
```python
_PRICE_CACHE: dict[str, tuple[float, float]] = {}  # symbol -> (price, expires_at)
_PRICE_CACHE_TTL = 5.0

def get_price_sync(symbol: str, base_url: str | None = None) -> dict[str, Any]:
    now = time.monotonic()
    cached = _PRICE_CACHE.get(symbol)
    if cached and cached[1] > now:
        return {"symbol": symbol.upper(), "price": cached[0]}
    # ... existing fetch logic ...
    if "price" in result:
        _PRICE_CACHE[symbol] = (float(result["price"]), now + _PRICE_CACHE_TTL)
    return result
```

This caps price lookups to at most 1 per 5 seconds per symbol, regardless of how many balance snapshots fire. Freshness is preserved (5s TTL is short enough for downstream services that poll balance info).

**Location**: `crypto_client.py`, `_price_for_asset` in `services_bitstamp.py`.

---

## Deployment & Monitoring

1. After deploying Fixes 1+2, monitor container RSS — should stabilize at 50-150 MB instead of climbing to GBs.
2. Check logs for "private websocket supervisor failed" frequency — if reconnects are frequent, Fix 3 is needed urgently.
3. Check SQLite file size (`data/exec_mcp.sqlite3`) — if growing, Fix 5 is needed.
4. Check `docker stats` for the container after 24-48 hours of running to confirm memory is flat.