Skip to content

Commit 6788a0d

Browse files
fix(room): release remote participants when the room is closed
`RoomSession::close` unpublished the local participant's tracks and then dropped the remote participant map. Dropping the map does not release the participants: every remote publication registers callbacks that hold the participant which owns them — `on_subscribed`/`on_unsubscribed` capture the `RemoteParticipant`, and `on_muted`/`on_unmuted` capture it through the publication map — so each remote participant is kept alive by a cycle through its own publications. A leaked participant holds `Arc<RtcEngine>`, so it pins the whole engine, its peer connections and the WebRTC runtime: exactly what commit 16530d2 set out to release. Each connect/close cycle leaked another one. `handle_participant_disconnect` already unregisters these when a peer actually leaves, but it also emits ParticipantDisconnected and TrackUnpublished. The peers have not left here — the room is closing — so unregister directly instead, leaving the event stream unchanged. Two ordering constraints make this correct: - The teardown runs after `room_handle` has been joined. `room_task` is what inserts remote participants, so draining the map any earlier would let an engine event queued behind the close repopulate it, and those participants would never be visited. - Each publication is unregistered before its track is detached. `RemoteTrackPublication::set_track(None)` invokes the unsubscribe handler *before* clearing it, and that handler dispatches TrackUnsubscribed carrying strong clones of the participant and track. Clearing the handlers first keeps teardown silent, so a receiver the application has stopped draining cannot pin the participant through a queued event. `test_close_releases_room_session` did not catch any of this because it runs with a single participant and no tracks, so neither cycle is ever formed. The new test uses two participants with a published, subscribed track, keeps its event receiver alive and undrained across the close, and asserts via a drop probe that the remote participant's internals are released.
1 parent 25931c7 commit 6788a0d

3 files changed

Lines changed: 103 additions & 4 deletions

File tree

livekit/src/room/mod.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1201,6 +1201,31 @@ impl RoomSession {
12011201
let _ = handle.remote_dt_task.await;
12021202
let _ = handle.room_handle.await;
12031203

1204+
// Tear down the remote participants. Dropping the map is not enough: each of their
1205+
// publications registers callbacks that hold the participant which owns them, so the
1206+
// participant — and the `Arc<RtcEngine>` it holds — survives the room unless those
1207+
// callbacks are unregistered.
1208+
//
1209+
// This runs only after `room_handle` has been joined. `room_task` is what inserts
1210+
// remote participants, so doing it any earlier would let an engine event queued
1211+
// behind us repopulate the map after we had already drained it, and those
1212+
// participants would never be visited.
1213+
//
1214+
// Unregister before detaching the track, not after: `RemoteTrackPublication::
1215+
// set_track(None)` invokes the unsubscribe handler *before* clearing, and that
1216+
// handler dispatches `TrackUnsubscribed` carrying strong clones of the participant
1217+
// and track. Clearing the handlers first keeps teardown silent, so a receiver the
1218+
// application has stopped draining cannot pin the participant through a queued
1219+
// event. `handle_participant_disconnect` is deliberately not reused for the same
1220+
// reason: these peers have not left, the room is closing.
1221+
let remote_participants = std::mem::take(&mut *self.remote_participants.write());
1222+
for participant in remote_participants.into_values() {
1223+
for (sid, publication) in participant.track_publications() {
1224+
participant.remove_publication(&sid);
1225+
publication.set_track(None);
1226+
}
1227+
}
1228+
12041229
self.dispatcher.clear();
12051230
Ok(())
12061231
}

livekit/src/room/participant/remote_participant.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,15 @@ impl RemoteParticipant {
112112
self.inner.track_publications.read().clone()
113113
}
114114

115+
/// Test-only: returns a probe that reports whether this participant's internals have
116+
/// been dropped. Each of its publications registers callbacks that hold the
117+
/// participant, so teardown has to unregister them; `Drop` alone never runs.
118+
#[cfg(feature = "__lk-e2e-test")]
119+
pub fn drop_probe(&self) -> impl Fn() -> bool + Send + Sync + 'static {
120+
let inner = Arc::downgrade(&self.inner);
121+
move || inner.upgrade().is_none()
122+
}
123+
115124
pub(crate) async fn add_subscribed_media_track(
116125
&self,
117126
sid: TrackSid,

livekit/tests/room_test.rs

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,15 @@
1414

1515
#[cfg(feature = "__lk-e2e-test")]
1616
use {
17-
anyhow::{Ok, Result},
17+
anyhow::{anyhow, Ok, Result},
1818
chrono::{TimeDelta, TimeZone, Utc},
19-
common::test_rooms,
19+
common::{
20+
test_rooms,
21+
video::{SolidColorParams, SolidColorTrack},
22+
},
2023
libwebrtc::prelude::PeerConnectionState,
21-
livekit::{ConnectionState, ParticipantKind, RoomEvent},
22-
std::time::Duration,
24+
livekit::{options::VideoCodec, ConnectionState, ParticipantKind, RoomEvent},
25+
std::{sync::Arc, time::Duration},
2326
tokio::time::{self, timeout},
2427
};
2528

@@ -140,3 +143,65 @@ async fn test_close_releases_room_session() -> Result<()> {
140143
assert!(session_dropped(), "room callbacks retained the room session after close");
141144
Ok(())
142145
}
146+
147+
/// `close()` must tear down remote participants, not only the local one.
148+
///
149+
/// Every remote publication registers callbacks that hold the participant which owns
150+
/// them: `on_subscribed`/`on_unsubscribed` capture the `RemoteParticipant`, and
151+
/// `on_muted`/`on_unmuted` capture it through the publication map. Those are reference
152+
/// cycles, so a remote participant — and the `Arc<RtcEngine>` it holds — outlives the
153+
/// room unless teardown unregisters them. Only the disconnect path does that, and
154+
/// `close()` never walks the remote participants.
155+
#[cfg(feature = "__lk-e2e-test")]
156+
#[test_log::test(tokio::test)]
157+
async fn test_close_releases_remote_participants() -> Result<()> {
158+
let mut rooms = test_rooms(2).await?;
159+
let (sub_room, mut sub_events) = rooms.remove(0);
160+
let (pub_room, _pub_events) = rooms.remove(0);
161+
162+
// The cycles only exist once the remote participant has a publication.
163+
let mut solid_track = SolidColorTrack::new(
164+
Arc::new(pub_room),
165+
SolidColorParams { width: 320, height: 240, luma: 128 },
166+
);
167+
solid_track.publish(VideoCodec::VP8, false).await?;
168+
169+
timeout(Duration::from_secs(15), async {
170+
loop {
171+
match sub_events.recv().await {
172+
Some(RoomEvent::TrackSubscribed { .. }) => break Ok(()),
173+
Some(_) => continue,
174+
None => break Err(anyhow!("event stream ended before the track was subscribed")),
175+
}
176+
}
177+
})
178+
.await??;
179+
180+
let remote = sub_room
181+
.remote_participants()
182+
.into_values()
183+
.next()
184+
.ok_or_else(|| anyhow!("subscriber never saw the publisher"))?;
185+
let remote_dropped = remote.drop_probe();
186+
drop(remote);
187+
188+
// `sub_events` is deliberately kept alive and undrained across the close. Teardown
189+
// must not dispatch anything carrying the participant: a queued event holds strong
190+
// clones, so a receiver the application has stopped polling would pin the participant
191+
// just as effectively as the callbacks did.
192+
sub_room.close().await?;
193+
drop(sub_room);
194+
195+
let released = timeout(Duration::from_secs(10), async {
196+
while !remote_dropped() {
197+
time::sleep(Duration::from_millis(50)).await;
198+
}
199+
})
200+
.await
201+
.is_ok();
202+
203+
drop(sub_events);
204+
205+
assert!(released, "remote participant retained after the room was closed");
206+
Ok(())
207+
}

0 commit comments

Comments
 (0)