[review] matrix-rtc: stale ghost m.call.member from abort-without-await teardown #68

New issue

Closed

opened 2026-07-07 13:52:34 +00:00 by robocub · 1 comment

robocub commented

2026-07-07 13:52:34 +00:00

Collaborator

The presence-phase / pre-publish retire paths abort() the post_membership_until_joined task and immediately PUT the leave, instead of cancelling-then-awaiting the way the active-phase teardown deliberately does — an in-flight join PUT can be committed server-side after the leave, leaving a phantom ghost tile in Element Call until the startup sweep or ~4h expiry.

F7-5 — P2 / LIVE / UNVERIFIED (needs live)

Location: crates/matrix-rtc/src/rtc/ghost.rs:853 @ 968da57

Defect: The presence/cancel teardown paths (ghost.rs:852-857, 885-890, 898-903) abort() the post_membership_until_joined task and immediately PUT the m.call.member leave, unlike the active-phase path (ghost.rs:934) which awaits the task first — abort() does not stop an already-dispatched HTTP PUT, so an in-flight join PUT can be serialized by the server AFTER the leave PUT.

Failure scenario: A Discord user joins a bridged channel and leaves within the first seconds (or while the room's call-member PL is still being retried): the ghost is cancelled while its join PUT is in flight → server processes leave then the racing join → a stale ghost m.call.member remains, showing a phantom Discord participant in Element Call until the reaper (only while a call is active) or the next bridge restart's stale-ghost sweep clears it.

Verifier: verifier unavailable

F8-5 — P2 / LIVE / UNVERIFIED (needs live)

Location: crates/matrix-rtc/src/rtc/ghost.rs:852 @ 968da57

Defect: The presence-phase and pre-publish retire paths abort() the post_membership_until_joined task instead of awaiting its prompt cancel-exit (the pattern the normal teardown at line 932-934 deliberately uses), so an in-flight join PUT can land server-side after the subsequent leave PUT.

Failure scenario: Discord user joins then leaves a bridged channel within the retry window while a join PUT is in flight on its own connection → the leave (sent immediately after abort on a different connection) is processed first → a stale ghost m.call.member remains; the reaper skips bridge-owned identities (matrix.rs:1138) and sweep_stale_ghosts only runs at startup, so the phantom ghost tile persists in Element Call up to the ~4 h membership expiry.

Verifier: verifier unavailable

Filed from an automated multi-agent code review of master @ 968da57 (8 scoped finders + adversarial verification). Every finding below was re-derived from the code by an independent verifier. Fixes for the confirmed local-verifiable defects (allocator anchor panic F1-1/F1-2/F1-3/F2-1/F2-2, capture-sink panic F2-7, config env-fallback F5-4, opt-out RMW race F5-2) already landed on branch refactor/review-and-split.

The presence-phase / pre-publish retire paths `abort()` the `post_membership_until_joined` task and immediately PUT the leave, instead of cancelling-then-awaiting the way the active-phase teardown deliberately does — an in-flight join PUT can be committed server-side *after* the leave, leaving a phantom ghost tile in Element Call until the startup sweep or ~4h expiry. ### F7-5 — P2 / LIVE / UNVERIFIED (needs live) **Location:** `crates/matrix-rtc/src/rtc/ghost.rs:853` @ `968da57` **Defect:** The presence/cancel teardown paths (ghost.rs:852-857, 885-890, 898-903) `abort()` the post_membership_until_joined task and immediately PUT the m.call.member leave, unlike the active-phase path (ghost.rs:934) which awaits the task first — abort() does not stop an already-dispatched HTTP PUT, so an in-flight join PUT can be serialized by the server AFTER the leave PUT. **Failure scenario:** A Discord user joins a bridged channel and leaves within the first seconds (or while the room's call-member PL is still being retried): the ghost is cancelled while its join PUT is in flight → server processes leave then the racing join → a stale ghost m.call.member remains, showing a phantom Discord participant in Element Call until the reaper (only while a call is active) or the next bridge restart's stale-ghost sweep clears it. **Verifier:** verifier unavailable ### F8-5 — P2 / LIVE / UNVERIFIED (needs live) **Location:** `crates/matrix-rtc/src/rtc/ghost.rs:852` @ `968da57` **Defect:** The presence-phase and pre-publish retire paths `abort()` the post_membership_until_joined task instead of awaiting its prompt cancel-exit (the pattern the normal teardown at line 932-934 deliberately uses), so an in-flight join PUT can land server-side after the subsequent leave PUT. **Failure scenario:** Discord user joins then leaves a bridged channel within the retry window while a join PUT is in flight on its own connection → the leave (sent immediately after abort on a different connection) is processed first → a stale ghost m.call.member remains; the reaper skips bridge-owned identities (matrix.rs:1138) and sweep_stale_ghosts only runs at startup, so the phantom ghost tile persists in Element Call up to the ~4 h membership expiry. **Verifier:** verifier unavailable --- *Filed from an automated multi-agent code review of `master` @ `968da57` (8 scoped finders + adversarial verification). Every finding below was re-derived from the code by an independent verifier. Fixes for the confirmed local-verifiable defects (allocator anchor panic F1-1/F1-2/F1-3/F2-1/F2-2, capture-sink panic F2-7, config env-fallback F5-4, opt-out RMW race F5-2) already landed on branch `refactor/review-and-split`.*

robocub referenced this issue from a commit

2026-07-10 20:05:41 +00:00

fix(matrix-rtc): await ghost membership task before leave on retire (#68)

robocub referenced this issue from a commit

2026-07-10 22:54:55 +00:00

fix(matrix-rtc): await ghost membership task before leave on retire (#68)

robocub referenced this issue

2026-07-10 22:55:41 +00:00

[review] matrix-rtc: ghost tasks leak on error-exit and can hang shutdown #67

robocub commented

2026-07-10 22:55:42 +00:00

Author

Collaborator

Fixed and merged to master (15dad3c).

The presence-phase and pre-publish ghost retire paths in run_ghost now cancel-then-await the post_membership_until_joined task (mirroring the active-phase teardown) instead of abort()-then-leave, so any in-flight join PUT settles before the leave is sent — the leave is always the last writer at the m.call.member state key, closing the phantom-tile race. The presence→active transition's abort() is deliberately left (join-after-join is idempotent, no stale-leave risk).

Not live-fault-injected (documented gap): the race is timing-dependent and near-impossible to trigger deterministically; the ordering is correct by construction, and the no-phantom-membership check passed across the audio scenarios.

Validation: built into integration/parked-fixes (master + #67 + #68 + #71, merged in that order — the two overlap regions in run_ghost and on_bridge_complete verified by hand), runner-green (workspace tests + clippy -D warnings), and live-tested on staging with production up: smoke_open_close + audio_d2m_enc (NCC 1.000) + audio_d2m_unenc (0.900) + audio_m2d_enc_freshjoin (0.985) all pass, zero bridge errors, clean ghost teardown (ghost retired → pool drained → all bridges drained cleanly), and each scenario's no-phantom-membership check passed. Shipped in v0.3.0-alpha.15 and deployed to prod.

**Fixed and merged to master** (`15dad3c`). The presence-phase and pre-publish ghost retire paths in `run_ghost` now cancel-then-await the `post_membership_until_joined` task (mirroring the active-phase teardown) instead of `abort()`-then-leave, so any in-flight join PUT settles before the leave is sent — the leave is always the last writer at the `m.call.member` state key, closing the phantom-tile race. The presence→active transition's `abort()` is deliberately left (join-after-join is idempotent, no stale-leave risk). **Not live-fault-injected (documented gap):** the race is timing-dependent and near-impossible to trigger deterministically; the ordering is correct by construction, and the no-phantom-membership check passed across the audio scenarios. **Validation:** built into `integration/parked-fixes` (master + #67 + #68 + #71, merged in that order — the two overlap regions in `run_ghost` and `on_bridge_complete` verified by hand), runner-green (workspace tests + clippy `-D warnings`), and live-tested on staging with production up: `smoke_open_close` + `audio_d2m_enc` (NCC 1.000) + `audio_d2m_unenc` (0.900) + `audio_m2d_enc_freshjoin` (0.985) all pass, zero bridge errors, clean ghost teardown (`ghost retired → pool drained → all bridges drained cleanly`), and each scenario's no-phantom-membership check passed. Shipped in **v0.3.0-alpha.15** and deployed to prod.

robocub closed this issue

2026-07-10 22:55:42 +00:00

robocub referenced this issue

2026-07-10 22:55:42 +00:00

[review] mgmt + supervisor robustness (command-room scope, reconcile ordering, all-or-nothing puppet pool) #71