Process call events one at a time #8

Merged
balint merged 1 commit from fix/serialize-call-events into main 2026-06-06 22:15:15 +01:00
Owner

Found while auditing for async cleanup races (follow-up to #7). This one isn't a cleanup race but a check-then-act re-entrancy that can misbehave at runtime.

Why

Events reach CallController.handle from independent sources: the button is dispatched as a fire-and-forget task (asyncio.ensure_future in PhoneApp._on_button), while incoming messages are handled on the signaling recv loop. handle() reads the current state, awaits an action, then transitions — so two overlapping events each act on the same pre-action state:

  • a fast double button-press both read CALLING and each send CANCEL;
  • a button press racing an incoming ANSWER can double-act or land in the wrong state.

Change

  • Guard handle() with an asyncio.Lock so each event's read → act → transition is atomic; the next event then sees the updated state. No action re-enters handle(), so there's no risk of self-deadlock.
  • Add a regression test firing two concurrent presses from CALLING: it sends CANCEL twice without the lock and exactly once with it.

🤖 Generated with Claude Code

Found while auditing for async cleanup races (follow-up to #7). This one isn't a cleanup race but a check-then-act re-entrancy that can misbehave at runtime. ## Why Events reach `CallController.handle` from independent sources: the button is dispatched as a fire-and-forget task (`asyncio.ensure_future` in `PhoneApp._on_button`), while incoming messages are handled on the signaling recv loop. `handle()` reads the current state, **`await`s an action**, then transitions — so two overlapping events each act on the same pre-action state: - a fast double button-press both read `CALLING` and each send `CANCEL`; - a button press racing an incoming `ANSWER` can double-act or land in the wrong state. ## Change - Guard `handle()` with an `asyncio.Lock` so each event's read → act → transition is atomic; the next event then sees the updated state. No action re-enters `handle()`, so there's no risk of self-deadlock. - Add a regression test firing two concurrent presses from `CALLING`: it sends `CANCEL` twice without the lock and exactly once with it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Events reach CallController from independent sources — the button arrives as a
fire-and-forget task (ensure_future) while incoming messages are handled on the
signaling recv loop — so handle() could run concurrently. It reads the current
state, awaits an action, then transitions, so two overlapping events each acted
on the same pre-action state: a fast double-press sent CANCEL twice, and a
button press racing an incoming ANSWER could double-act or land in the wrong
state.

Guard handle() with an asyncio.Lock so each event's read-act-transition runs
atomically and the next event sees the updated state. Add a regression test
driving two concurrent presses that sends CANCEL twice without the lock.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
balint merged commit 69e67bf791 into main 2026-06-06 22:15:15 +01:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
balint/phone!8
No description provided.