From b9803f93fb9778eba8882556431d50e13157faf5 Mon Sep 17 00:00:00 2001 From: Mollusk Date: Fri, 31 Jul 2026 04:05:17 -0400 Subject: [PATCH] core: retire the share at Join's session teardown, not after ticket parse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gemini's review of the S2 branch found the one hole the harness had not covered (P2, verified reachable): Join tears the old session down — deliberately killing the share host — BEFORE validating the ticket, and an invalid ticket exits the arm early, skipping the late `current_sharing = None`. The killed host's stdout EOF then passed the staleness gate and the user got a spurious "Screen share ended unexpectedly" on top of "invalid room ticket". Pre-S2 the stale value was toothless on this path; the fault handler gave it teeth. The share now dies where the session does: cleared unconditionally right after the teardown block, ahead of every early exit. The live gate grew a third half — share, Join with a garbage ticket, then require silence after the ticket error — and the mutant restoring the old placement is killed by exactly that assertion (spurious re-emitted ScreenShareStopped). Also Gemini's P3: the test's temp dir is now dropped by a guard, so an assertion panic no longer leaks the fake-pixelpass scripts in /tmp. Co-Authored-By: Claude Fable 5 --- src/core/mod.rs | 11 +++++- tests/screenshare_host_fault.rs | 66 +++++++++++++++++++++++++++++++-- 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/src/core/mod.rs b/src/core/mod.rs index 58dcbdd..248a03b 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -1733,6 +1733,13 @@ async fn run_core_loop( net.file_router.clear(); *current_room.lock().unwrap() = None; } + // Any advertised share died with that session — deliberately — + // so retire it HERE, before the invalid-ticket early exit below + // can skip it. Left populated, the killed host's stdout EOF + // would pass the ScreenShareHostFault staleness gate and + // surface as a spurious "ended unexpectedly" error on top of + // the ticket error (Gemini review of S2, P2-1). + current_sharing = None; // If a network-mode / identity change was deferred while a call was // active, rebuild the persistent stack now — after the old session is @@ -1828,8 +1835,8 @@ async fn run_core_loop( secret_key.clone(), )); - // Fresh join starts not sharing; clear any stale share ticket. - current_sharing = None; + // (The share was already retired beside the session teardown + // above; a fresh join starts not sharing.) let self_state = presence.to_state(is_muted.load(Ordering::Relaxed), endpoint.addr(), None); diff --git a/tests/screenshare_host_fault.rs b/tests/screenshare_host_fault.rs index 4700fd1..a0e2fe2 100644 --- a/tests/screenshare_host_fault.rs +++ b/tests/screenshare_host_fault.rs @@ -29,6 +29,16 @@ const EVENT_TIMEOUT: Duration = Duration::from_secs(20); /// be mishandled has arrived by the end of it. const QUIET_WINDOW: Duration = Duration::from_secs(3); +/// Removes the fake-pixelpass dir even when an assertion panics mid-test +/// (a plain trailing `remove_dir_all` never runs on an unwind). +struct TempDir(PathBuf); + +impl Drop for TempDir { + fn drop(&mut self) { + std::fs::remove_dir_all(&self.0).ok(); + } +} + fn write_fake_pixelpass(dir: &std::path::Path, name: &str, body: &str) -> PathBuf { let path = dir.join(name); std::fs::write(&path, body).expect("write fake pixelpass"); @@ -60,7 +70,9 @@ async fn wait_for( #[tokio::test] #[ignore = "live: joins a real solo room (audio backend + network bind)"] async fn a_dead_host_is_torn_down_and_a_clean_stop_stays_clean() { - let dir = std::env::temp_dir().join(format!("peerspeak-hostfault-{}", std::process::id())); + let dir_guard = + TempDir(std::env::temp_dir().join(format!("peerspeak-hostfault-{}", std::process::id()))); + let dir = dir_guard.0.clone(); std::fs::create_dir_all(&dir).unwrap(); // Half 1's host: emits its ticket, then dies on its own — the S2 defect @@ -192,8 +204,56 @@ async fn a_dead_host_is_torn_down_and_a_clean_stop_stays_clean() { } } - assert!(controller.send(CoreCommand::Leave)); - std::fs::remove_dir_all(&dir).ok(); + // ── Half 3: a failed room switch while sharing must not cry "crash" ───── + // Join tears the old session down (killing the host, deliberately) BEFORE + // it validates the ticket, so an invalid ticket exits the Join arm early. + // The share must be retired at the teardown itself — left advertised, the + // killed host's EOF passes the staleness gate and a spurious "ended + // unexpectedly" lands on top of the ticket error (Gemini review, P2-1). + assert!(controller.send(CoreCommand::StartScreenShare { + audio_app: None, + settings: Default::default(), + quality: Default::default(), + })); + wait_for( + &mut ui_rx, + "ScreenShareStarted (before failed switch)", + |ev| match ev { + UiEvent::ScreenShareStarted => Some(()), + UiEvent::Error(e) => panic!("third share start failed: {e}"), + _ => None, + }, + ) + .await; + assert!(controller.send(CoreCommand::Join { + name: "host-fault-gate".into(), + ticket: "definitely-not-a-ticket".into(), + room_name: "s2".into(), + input_device: None, + output_device: None, + echo_cancellation: false, + avatar: Default::default(), + })); + wait_for(&mut ui_rx, "the invalid-ticket error", |ev| match ev { + UiEvent::Error(e) if e.contains("invalid room ticket") => Some(()), + UiEvent::Error(e) => panic!("unexpected error before the ticket error: {e}"), + _ => None, + }) + .await; + // The deliberately-killed host's EOF is arriving about now; it must be + // dropped as stale, not reported as a crash. + let deadline = tokio::time::Instant::now() + QUIET_WINDOW; + while let Ok(Some(ev)) = tokio::time::timeout_at(deadline, ui_rx.recv()).await { + match ev { + UiEvent::ScreenShareStopped => { + panic!("failed room switch re-emitted ScreenShareStopped for the torn-down share") + } + UiEvent::Error(e) if e.contains("unexpectedly") => { + panic!("deliberate teardown during a failed room switch reported as a crash: {e}") + } + _ => {} + } + } } /// The long-owed Stop Share SIGINT gate (0c half (ii)), against the REAL