core: retire the share at Join's session teardown, not after ticket parse
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 <noreply@anthropic.com>
This commit is contained in:
+9
-2
@@ -1733,6 +1733,13 @@ async fn run_core_loop(
|
|||||||
net.file_router.clear();
|
net.file_router.clear();
|
||||||
*current_room.lock().unwrap() = None;
|
*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
|
// If a network-mode / identity change was deferred while a call was
|
||||||
// active, rebuild the persistent stack now — after the old session is
|
// active, rebuild the persistent stack now — after the old session is
|
||||||
@@ -1828,8 +1835,8 @@ async fn run_core_loop(
|
|||||||
secret_key.clone(),
|
secret_key.clone(),
|
||||||
));
|
));
|
||||||
|
|
||||||
// Fresh join starts not sharing; clear any stale share ticket.
|
// (The share was already retired beside the session teardown
|
||||||
current_sharing = None;
|
// above; a fresh join starts not sharing.)
|
||||||
let self_state =
|
let self_state =
|
||||||
presence.to_state(is_muted.load(Ordering::Relaxed), endpoint.addr(), None);
|
presence.to_state(is_muted.load(Ordering::Relaxed), endpoint.addr(), None);
|
||||||
|
|
||||||
|
|||||||
@@ -29,6 +29,16 @@ const EVENT_TIMEOUT: Duration = Duration::from_secs(20);
|
|||||||
/// be mishandled has arrived by the end of it.
|
/// be mishandled has arrived by the end of it.
|
||||||
const QUIET_WINDOW: Duration = Duration::from_secs(3);
|
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 {
|
fn write_fake_pixelpass(dir: &std::path::Path, name: &str, body: &str) -> PathBuf {
|
||||||
let path = dir.join(name);
|
let path = dir.join(name);
|
||||||
std::fs::write(&path, body).expect("write fake pixelpass");
|
std::fs::write(&path, body).expect("write fake pixelpass");
|
||||||
@@ -60,7 +70,9 @@ async fn wait_for<T>(
|
|||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
#[ignore = "live: joins a real solo room (audio backend + network bind)"]
|
#[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() {
|
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();
|
std::fs::create_dir_all(&dir).unwrap();
|
||||||
|
|
||||||
// Half 1's host: emits its ticket, then dies on its own — the S2 defect
|
// 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));
|
// ── Half 3: a failed room switch while sharing must not cry "crash" ─────
|
||||||
std::fs::remove_dir_all(&dir).ok();
|
// 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
|
/// The long-owed Stop Share SIGINT gate (0c half (ii)), against the REAL
|
||||||
|
|||||||
Reference in New Issue
Block a user