Skip to content

Room::close() is not cancellation-safe — cancelling it leaks the room's ICE UDP sockets #1334

Description

@sebitokazu

If a Room::close() future is cancelled, or a Room is dropped without closing, the room's ICE UDP sockets are never released. Long-lived clients end up at EMFILE. Ours dies after about 6 hours on a Raspberry Pi Zero 2 W.

SessionInner::close calls publisher_pc.close() — the only thing that frees the sockets — after two awaits that can block indefinitely:

self.signal_client.send(Leave { .. }).await;
self.signal_client.close().await;
self.publisher_pc.close();

SignalStream::send waits on a writer task that does conn.send(data).await with no timeout, so a half-open socket parks it until TCP gives up (~15 min with tcp_retries2=15, or never against a zero-window peer). SignalStream::close then joins that same parked task, and the InternalMessage::Close it queues sits behind the stuck message in a capacity-8 channel, so the task never breaks its loop. There is no .abort() anywhere in the chain, only joins.

Every timeout in signal_client is on the connect path (SIGNAL_CONNECT_TIMEOUT, JOIN_RESPONSE_TIMEOUT, VALIDATE_TIMEOUT, REGION_FETCH_TIMEOUT). The teardown path has none.

Dropping the Room is not a fallback either, though not for the reason it first appears. The native destructor is fine: a bare libwebrtc::PeerConnection that gathers and is then dropped without close() returns every socket (measured: +4 UDP gathered, −4 returned, 0 retained — identical to closing first). The problem is that the graph never drops. engine_task(self: Arc<Self>, ..) and room_task(self: Arc<Self>, ..) own Arcs of the inner state and exit only on close_rx, and there is no impl Drop for Room/RoomSession/RtcEngine/RtcSession. An un-closed room leaves those tasks looping forever, so the refcount never reaches zero and the destructor never runs.

Measured on aarch64, livekit 0.7.52, bare connect/close with no tracks published or subscribed. Descriptors counted by intersecting /proc/self/fd socket inodes with /proc/self/net/udp{,6}:

teardown UDP leaked per cycle
close() awaited 0
close() cancelled ~13
Room dropped ~13

Repro:

#[tokio::main]
async fn main() {
    let (url, key, secret) = (env("LIVEKIT_URL"), env("LIVEKIT_API_KEY"), env("LIVEKIT_API_SECRET"));
    for i in 1..=10 {
        let token = AccessToken::with_api_key(&key, &secret)
            .with_identity("probe")
            .with_grants(VideoGrants { room_join: true, room: format!("probe-{i}"), ..Default::default() })
            .to_jwt().unwrap();
        let (room, _rx) = Room::connect(&url, &token, RoomOptions::default()).await.unwrap();
        tokio::time::sleep(Duration::from_secs(2)).await;
        // 10 ms only to make the cancellation deterministic; any expiring budget leaks the same.
        let _ = tokio::time::timeout(Duration::from_millis(10), room.close()).await;
        println!("iter {i}: fds={} udp={}", fd_count(), udp_count());
    }
}

Swap the timeout for room.close().await and the leak goes to zero.

Suggested fix

Move the peer-connection close above the awaits. PeerTransport::close is synchronous, so running it before the first suspension point makes it unskippable — no cancellation of this future, at any point, can leave the peer connections open. Closing them does not touch the signalling socket, so the Leave still goes out and teardown semantics are unchanged.

 async fn close(&self, reason: DisconnectReason) {
     self.closed.store(true, Ordering::Release);
     self.pc_state_notify.notify_waiters();
+
+    self.publisher_pc.close();
+    if let Some(ref sub_pc) = self.subscriber_pc {
+        sub_pc.close();
+    }
 
     self.signal_client
         .send(proto::signal_request::Message::Leave(proto::LeaveRequest {
             action: proto::leave_request::Action::Disconnect.into(),
             reason: reason as i32,
             ..Default::default()
         }))
         .await;
 
     self.signal_client.close().await;
-    self.publisher_pc.close();
-    if let Some(ref sub_pc) = self.subscriber_pc {
-        sub_pc.close();
-    }
 }

Verified against a patched 0.7.52 on the same hardware:

repro mode before after
close() cancelled at 10 ms ~13 UDP/cycle 0
close() awaited 0 0
Room dropped ~13 UDP/cycle ~12 (unchanged)

The cancelled run still logged the cancellation on every iteration, so the abandonment genuinely fired and the leak went to zero regardless. Room-dropped is unchanged, which is expected — that path needs the tasks stopped, and is a separate change.

For reference, client-sdk-js already orders it this way: RTCEngine.close() closes the peer connections before the signal client.

Verified unchanged from 0.7.52 through 0.8.3 and on main. libwebrtc 0.3.43, webrtc-sys 0.3.40, aarch64-unknown-linux-gnu, glibc 2.28.

Happy to open a PR with this.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions