Fix stale DLNA SessionEnded tearing down the next cast session (CAST-4) #5

Closed

Breadway wants to merge 0 commits from fix/cast-4-dlna-stale-session-ended into main

Breadway commented

2026-09-05 02:19:16 +08:00

Owner

Fixes CAST-4.

Root cause

Two compounding bugs, both confirmed against the real code:

daemon.rs's StopCast handler only bumped start_generation
when it cancelled an in-flight start (self.starting). When it tore
down an already-installed session (the ordinary "user clicks Stop"
path), the generation was left unchanged. A stop, then a quick start,
kept the exact same generation number throughout.
dlna_mirror.rs's bus-watcher thread (spawned via bare
std::thread::spawn around what's now start()'s bus_watcher
block) had its JoinHandle discarded — not stored on
DlnaMirrorSession, never joined in stop(). So stop() could
return before that thread noticed the pipeline reached Null and
sent its SessionEnded { generation }, an unbounded (if usually
short) time later.

Combined: a stop → start on DLNA within a few seconds could let the old
session's late SessionEnded — carrying the unchanged generation —
either cancel the new session's in-flight start, or active_session.take()
and tear down the new session outright, right after it started
successfully mirroring.

Fix (both parts, per the issue's own analysis)

breadcastd/src/daemon.rs: StopCast now bumps start_generation
whenever it takes an installed active_session, mirroring what the
SessionEnded handler already does when a session ends on its own.
This alone makes the generation check correctly reject any late
message from a superseded session, since the generation is captured by
value in the watcher thread at spawn time.
breadcastd/src/dlna_mirror.rs: the bus-watcher thread's JoinHandle
is now stored on DlnaMirrorSession and joined (bounded to 5s) in
stop(), right after the pipeline is set to Null (which is what
unblocks it) — the same pattern CastMirrorSession::stop() already
uses for frame_pump. This closes the resource-leak/non-determinism
gap and brings DLNA to parity with the Cast path, so stop() is a real
synchronization point again.

Testing

Added a deterministic regression test in daemon.rs
(stop_cast_on_an_installed_session_bumps_generation_so_a_stale_watcher_thread_cannot_kill_the_next_session)
that drives the exact stop → install-new-session → stale-SessionEnded
sequence directly through Daemon::handle, using a #[cfg(test)]-only
ActiveSession::Stub variant (a real session needs a portal capture
session + GStreamer pipeline, so it can't be constructed in a unit test —
same constraint the existing in-flight-start race tests already work
around). No sleeps, no real threads, nothing timing-dependent.

cargo build --release --workspace   # ok
cargo test --release --workspace    # ok, 17 passed / 0 failed
cargo clippy --release --workspace --all-targets -- -D warnings   # clean

Fixes CAST-4. ## Root cause Two compounding bugs, both confirmed against the real code: 1. **`daemon.rs`'s `StopCast` handler** only bumped `start_generation` when it cancelled an *in-flight* start (`self.starting`). When it tore down an already-*installed* session (the ordinary "user clicks Stop" path), the generation was left unchanged. A stop, then a quick start, kept the exact same generation number throughout. 2. **`dlna_mirror.rs`'s bus-watcher thread** (spawned via bare `std::thread::spawn` around what's now `start()`'s `bus_watcher` block) had its `JoinHandle` discarded — not stored on `DlnaMirrorSession`, never joined in `stop()`. So `stop()` could return before that thread noticed the pipeline reached `Null` and sent its `SessionEnded { generation }`, an unbounded (if usually short) time later. Combined: a stop → start on DLNA within a few seconds could let the old session's late `SessionEnded` — carrying the *unchanged* generation — either cancel the new session's in-flight start, or `active_session.take()` and tear down the new session outright, right after it started successfully mirroring. ## Fix (both parts, per the issue's own analysis) - `breadcastd/src/daemon.rs`: `StopCast` now bumps `start_generation` whenever it takes an installed `active_session`, mirroring what the `SessionEnded` handler already does when a session ends on its own. This alone makes the generation check correctly reject *any* late message from a superseded session, since the generation is captured by value in the watcher thread at spawn time. - `breadcastd/src/dlna_mirror.rs`: the bus-watcher thread's `JoinHandle` is now stored on `DlnaMirrorSession` and joined (bounded to 5s) in `stop()`, right after the pipeline is set to `Null` (which is what unblocks it) — the same pattern `CastMirrorSession::stop()` already uses for `frame_pump`. This closes the resource-leak/non-determinism gap and brings DLNA to parity with the Cast path, so `stop()` is a real synchronization point again. ## Testing Added a deterministic regression test in `daemon.rs` (`stop_cast_on_an_installed_session_bumps_generation_so_a_stale_watcher_thread_cannot_kill_the_next_session`) that drives the exact stop → install-new-session → stale-`SessionEnded` sequence directly through `Daemon::handle`, using a `#[cfg(test)]`-only `ActiveSession::Stub` variant (a real session needs a portal capture session + GStreamer pipeline, so it can't be constructed in a unit test — same constraint the existing in-flight-start race tests already work around). No sleeps, no real threads, nothing timing-dependent. ``` cargo build --release --workspace # ok cargo test --release --workspace # ok, 17 passed / 0 failed cargo clippy --release --workspace --all-targets -- -D warnings # clean ```

Breadway added 1 commit

2026-09-05 02:19:16 +08:00

Fix stale DLNA SessionEnded tearing down the next cast session (CAST-4) d6021fe2db

StopCast only bumped start_generation when it cancelled an in-flight
start, not when it tore down an already-installed session -- so a
stop -> start sequence on an installed session left the generation
unchanged, and a late SessionEnded from the stopped session's own
background thread/task could be mistaken for one about whatever gets
started next. Bump the generation unconditionally whenever StopCast
takes an installed session, mirroring what the SessionEnded handler
already does when a session ends on its own.

DLNA's bus-watcher thread also spawned untracked and unjoined -- its
JoinHandle was discarded, so stop() had no guarantee the thread (and
its SessionEnded send) had finished before returning, unlike
CastMirrorSession's frame_pump, which is stored and joined (bounded)
in stop(). Store the JoinHandle and join it the same way, once the
pipeline reaching Null unblocks it.

Added a deterministic regression test in daemon.rs that drives the
stop -> install-new-session -> stale-SessionEnded sequence directly
through Daemon::handle, with no real session or timing involved.

Breadway referenced this pull request

2026-09-05 02:19:29 +08:00

Stale DLNA SessionEnded from an untracked bus-watcher thread can kill (or wrongly tear down) the next cast #4

Breadway closed this pull request

2026-09-10 10:00:44 +08:00