From 73007cb38e1fd688543d14b591ab788e0a6384a3 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Sat, 8 Aug 2026 13:10:04 +0800 Subject: [PATCH] webrtc: split the send budget, make close_webrtc uncancellable, restore log retention - send_bytes computed one deadline before acquiring the send gate and reused it for the write, so time queued behind another clone's message was charged to this message. A caller that only just lost the gate race got a few milliseconds to write in and then tore the whole peer connection down for missing them - the narrower the miss, the more certain the teardown. The write gets its own budget; the gate wait keeps the one it had, and still never closes (it never held the permit). - Stream::close_webrtc awaited WebRTCStream::close, which is exactly the form close_detached exists to avoid: most callers sit in a select! arm or a future the UI can abandon, and a cancelled close is unretryable (is_closed is latched before the first await, so later attempts early-return and the handler that evicts the session never runs). It is now a plain fn calling close_detached - with no await point there is nothing to cancel. close_detached is public and carries the runtime-teardown guard the parent repo had written separately for its Drop path, so that copy goes away. - Log retention goes back to 31 files. Raising it to 31*8 defended the flood case badly (a file count cannot outrun a flood; only the rate limits at the log sites can) while silently multiplying steady-state retention and disk for every ordinary install, on every platform. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01ExUfAkYbq8UC9pQCiLy8TQ --- src/lib.rs | 14 +++++++------- src/stream.rs | 10 ++++++++-- src/webrtc.rs | 41 ++++++++++++++++++++++++++++++----------- 3 files changed, 45 insertions(+), 20 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 695c378c0..3e7435e57 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -464,13 +464,13 @@ pub fn init_log(_is_async: bool, _name: &str) -> Option s.close().await, + Stream::WebRTC(s) => s.close_detached(), #[allow(unreachable_patterns)] _ => {} } diff --git a/src/webrtc.rs b/src/webrtc.rs index 7ddc12f9f..fdb7297be 100644 --- a/src/webrtc.rs +++ b/src/webrtc.rs @@ -928,18 +928,31 @@ impl WebRTCStream { /// `keep` carries anything whose lifetime must span the teardown rather than the caller's: /// the send path passes its logical-message permit, so no waiting clone can append to a /// partially-written fragment sequence while the close is still in flight. - fn close_detached_with(&self, keep: T) { + pub fn close_detached_with(&self, keep: T) { let pc = self.pc.clone(); - tokio::spawn(async move { - let _keep = keep; - if let Err(err) = pc.close().await { - log::debug!("WebRTC background close failed: {}", err); - } - }); + // Take the runtime handle explicitly rather than calling `tokio::spawn`: this also runs + // from `Drop` impls, which can execute during runtime teardown where a bare spawn panics. + // `Handle::spawn` can panic while the runtime is shutting down too, so catch it — a brief + // leak until process exit beats aborting the process from a destructor. + let Ok(handle) = tokio::runtime::Handle::try_current() else { + log::warn!("no tokio runtime available to close the WebRTC peer connection"); + return; + }; + let spawned = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + handle.spawn(async move { + let _keep = keep; + if let Err(err) = pc.close().await { + log::debug!("WebRTC background close failed: {}", err); + } + }); + })); + if spawned.is_err() { + log::warn!("failed to spawn the WebRTC close (runtime shutting down)"); + } } #[inline] - fn close_detached(&self) { + pub fn close_detached(&self) { self.close_detached_with(()); } @@ -1041,8 +1054,8 @@ impl WebRTCStream { .await .map_err(|_| Error::new(ErrorKind::TimedOut, "WebRTC connect timeout"))??; - let deadline = Instant::now() + Duration::from_millis(send_timeout); - let send_permit = match timeout_at(deadline, send_gate.acquire_owned()).await { + let gate_deadline = Instant::now() + Duration::from_millis(send_timeout); + let send_permit = match timeout_at(gate_deadline, send_gate.acquire_owned()).await { Ok(Ok(permit)) => permit, Ok(Err(err)) => { return Err(Error::new( @@ -1059,7 +1072,13 @@ impl WebRTCStream { return Err(Error::new(ErrorKind::TimedOut, "WebRTC send gate timeout").into()); } }; - match timeout_at(deadline, self.send_bytes_inner(bytes)).await { + // Fresh budget once the permit is held: queueing behind another clone's message is + // not this message's progress, so charging it here meant a caller that only just lost + // the gate race got a few milliseconds to write in — and then tore the whole peer + // connection down for missing them. The narrower the miss, the more certain the + // teardown, which is precisely backwards. + let write_deadline = Instant::now() + Duration::from_millis(send_timeout); + match timeout_at(write_deadline, self.send_bytes_inner(bytes)).await { Ok(res) => res, Err(_) => { // Hand the logical-message permit to the teardown so no waiting clone can