mirror of
https://github.com/rustdesk/hbb_common.git
synced 2026-08-27 04:37:35 +00:00
webrtc/tests: close the stream a lost cancellation hands back
`test_cancelled_new_does_not_leak_the_pc` cancelled `new()` with a zero timeout and discarded whatever came back. The cancellation is not guaranteed to win: the setup task runs on WEBRTC_RT, and it can finish inside the single poll the timeout allows, in which case `new()` returns a live stream. `WebRTCStream` has no `Drop`, so `let _ =` on that one strands its pc in SESSIONS — and the test then reported the leak it had just created, blaming the cancelled attempt. Fewer test threads leave more CPU for that task, so it won the race often enough that `--test-threads=2` failed every run while the default count passed; the entry that survived carried `conn=New`, `sig=HaveLocalOffer` and a `Pending` state watch, i.e. a pc nobody had ever closed. Close what a lost race hands back and retry for a real cancellation, asserting that one happened rather than testing nothing. 24 tests now pass at 1, 2, 4, 8 and default threads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019UzcMTdYTEv2QbMHcTSUy3
This commit is contained in:
+19
-2
@@ -2256,8 +2256,25 @@ IHR5cCBzcmZseCByYWRkciAwLjAuMC4wIHJwb3J0IDY0MDA4XHJcbmE9ZW5kLW9mLWNhbmRpZGF0ZXNc
|
||||
async fn test_cancelled_new_does_not_leak_the_pc() {
|
||||
use std::collections::HashSet;
|
||||
let before: HashSet<String> = SESSIONS.lock().await.keys().cloned().collect();
|
||||
// Zero timeout: polls the future exactly once (spawning new_inner), then cancels it.
|
||||
let _ = timeout(Duration::ZERO, WebRTCStream::new("", false, 20000)).await;
|
||||
// Zero timeout: polls the future exactly once (spawning new_inner), then cancels it —
|
||||
// usually. The setup task runs on its own runtime and can finish inside that single poll,
|
||||
// and then `new()` hands back a live stream instead. `WebRTCStream` has no `Drop`, so
|
||||
// discarding that one is itself a leak, and this test would go on to report it as the
|
||||
// cancelled attempt's. Close what comes back and try for a real cancellation. Fewer test
|
||||
// threads make the setup task likelier to win, which is why this surfaced under
|
||||
// `--test-threads=2` and not at the default.
|
||||
let mut cancelled = false;
|
||||
for _ in 0..20 {
|
||||
match timeout(Duration::ZERO, WebRTCStream::new("", false, 20000)).await {
|
||||
Err(_) => {
|
||||
cancelled = true;
|
||||
break;
|
||||
}
|
||||
Ok(Ok(stream)) => stream.close().await,
|
||||
Ok(Err(_)) => {}
|
||||
}
|
||||
}
|
||||
assert!(cancelled, "new() never lost the race with its own cancellation");
|
||||
// Let the detached setup task finish (and insert its session) before sampling, or the
|
||||
// first sample is taken before the leak has formed and every later one intersects to
|
||||
// nothing.
|
||||
|
||||
Reference in New Issue
Block a user