• Stefy Lanza (nextime / spora )'s avatar
    broker: a swallowed cancel spun until the host died (v0.3.5) · 63efd6cf
    Stefy Lanza (nextime / spora ) authored
    A full `pytest tests/` run reached 46 GB RSS plus 20 GB of swap on a 54 GB
    machine. Swap filled, the OOM killer took coderai-nvidia — which was serving —
    and the box needed a hard reset, which lost the last forty minutes of
    kern.log with it. atop's history is what identified the process: a pytest
    worker, carrying the `coderai-front` process title because importing the front
    sets it, growing 7.8 GB → 34.8 GB → 46.6 GB across three ten-minute samples.
    
    The defect is `except asyncio.CancelledError: pass` wrapped around an await
    whose job is to collect a CHILD task. asyncio delivers a cancellation as a
    CancelledError at the next await point, so that handler catches two unrelated
    things: the child reporting it stopped (fine to swallow) and the caller asking
    us to stop (never fine). run_forever's reconnect handler sat on exactly such an
    await, so `task.cancel()` was absorbed and the loop carried on. With the
    reconnect delay mocked to zero by the test driving it, that became a busy loop
    allocating ~78 MB/s — AsyncMock records every call it receives — and 400,000
    iterations in the five seconds I measured.
    
    being_cancelled() (Task.cancelling(), 3.11+) tells the two apart, and all four
    sites now re-raise ours: _stop_heartbeat_task, _cancel_inflight_tasks, the two
    keepalive collects in handle_message, and BrokerService.stop. The inflight ones
    matter in production independently of the leak — absorbing a cancel there means
    a shutdown waits out a whole video generation instead of stopping it, and
    run_forever simply reconnected forever instead of exiting. run_forever's own
    cancellation path clears the cancel for the duration of its cleanup and
    re-raises at the end, so honouring the cancel does not cost the websocket close.
    
    The test that drove it now brakes itself: the zero-delay sleep stand-in counts,
    and raises a BaseException (anything else is caught by the reconnect handler)
    well past the two reconnects it needs. With the product bug reintroduced the
    file fails in 1.5 s at 441 MB instead of eating the machine.
    
    tests/conftest.py adds what was missing in general: an RSS ceiling (6 GB,
    CODERAI_TEST_RSS_LIMIT_MB) and a per-test stall ceiling (180 s,
    CODERAI_TEST_STALL_SECONDS), both dumping every thread's stack to a file before
    exiting — pytest captures stderr per test and drops the buffer when the process
    does not unwind, so a watchdog that only printed would have died silently. No
    new dependency. "A test can consume the host" is a property of the run, not of
    one test.
    
    tests/test_broker_protocol.py could not finish before this — it hung at the
    14th test. The whole suite now completes for the first time: 78 s, 1.57 GB peak,
    1635 passed. The 25 failures are the 17 that already failed plus the 8 stale
    register-ack expectations in that file, which were unreachable behind the hang
    and are a separate question (the fixtures send `accepted: true`; the client has
    required `status: "ok"` for some time).
    Co-Authored-By: 's avatarClaude Opus 5 <noreply@anthropic.com>
    Claude-Session: https://claude.ai/code/session_019sRvvg5WSdLgcFUM96pmE4
    63efd6cf
conftest.py 5.75 KB