Skip to content

Commit e296159

Browse files
authored
Ensure participant disconnects are synthesized after connection resume (#1250)
### Before you submit your PR Make sure the following is true before submitting your PR: - [ ] I have read the [contributing guidelines](https://github.com/livekit/rust-sdks/blob/main/CONTRIBUTING.md) and validated that this PR will be accepted. - [ ] I have read and followed the principles regarding breaking changes, testing, and code quality. ### PR description Describe the changes in this PR. Explain what the PR is meant to solve and how to reproduce the issue in the first place. ### Breaking changes If this PR introduces breaking changes, list them here and document the rationale for introducing such a change. ### MSRV If the PR modifies the crate's MSRV (Minimum Supported Rust Version), document it here. ### Testing Ideally, unit test the code you add, but ensure you're not repeating existing test cases. Use as many already written scaffolding, utilities as possible; write your own, when needed. If external services, APIs, tokens are required (e.g., running an LK server instance), provide the necessary information. Make sure your tests perform useful, context-aware assertions and do not simply emulate "happy paths". ### Async We want the project to be runtime-agnostic, so please reuse what's already in [livekit-runtime](https://github.com/livekit/rust-sdks/blob/main/livekit-runtime/) and feel free to add anything missing. It's ok to use Tokio directly, when writing unit tests, if necessary. When testing, do not use artificial delays for the state to "catch up"; instead, respect the event flow and subscribe properly using channels or other mechanisms.
1 parent acff35f commit e296159

6 files changed

Lines changed: 279 additions & 2 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
livekit: patch
3+
livekit-ffi: patch
4+
---
5+
6+
Ensure participant disconnects are synthesized after connection resume - #1250 (@lukasIO)

livekit/specs/signalling-reconnection.allium

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,12 @@ rule ResumeDrainsQueue {
565565
ensures: ResumeAttemptSucceeded(engine)
566566
@guidance
567567
-- Step 6: the resume has fully recovered; drain the signal queue.
568+
-- spec<->code: `resume_finalize` also hands the Room (external
569+
-- boundary) the participant identities seen since the resume began —
570+
-- a superset of the server's post-resume snapshot — and the Room
571+
-- synthesizes disconnects for known participants absent from it,
572+
-- whose DISCONNECTED update died with the previous connection.
573+
-- Participant state itself is below this spec's altitude.
568574
}
569575

570576
-- === Engine: attempt outcomes ===============================================

livekit/src/room/mod.rs

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,12 @@ use livekit_runtime::JoinHandle;
3737
use parking_lot::RwLock;
3838
pub use proto::DisconnectReason;
3939
use proto::SignalTarget;
40-
use std::{collections::HashMap, fmt::Debug, sync::Arc, time::Duration};
40+
use std::{
41+
collections::{HashMap, HashSet},
42+
fmt::Debug,
43+
sync::Arc,
44+
time::Duration,
45+
};
4146
use thiserror::Error;
4247
use tokio::sync::{
4348
broadcast,
@@ -847,6 +852,15 @@ impl Room {
847852
self.inner.rtc_engine.fail_transport_during_next_resume();
848853
}
849854

855+
/// Test-only: drop incoming DISCONNECTED participant entries, simulating an
856+
/// SFU that fails to (re)deliver disconnect updates (e.g. a resume served
857+
/// without the previous connection's state). Exercises the resume-time
858+
/// participant reconciliation.
859+
#[cfg(feature = "__lk-e2e-test")]
860+
pub fn drop_disconnected_updates(&self, enabled: bool) {
861+
self.inner.rtc_engine.drop_disconnected_updates(enabled);
862+
}
863+
850864
pub async fn get_stats(&self) -> EngineResult<SessionStats> {
851865
self.inner.rtc_engine.get_stats().await
852866
}
@@ -975,6 +989,9 @@ impl RoomSession {
975989
async fn on_engine_event(self: &Arc<Self>, event: EngineEvent) -> RoomResult<()> {
976990
match event {
977991
EngineEvent::ParticipantUpdate { updates } => self.handle_participant_update(updates),
992+
EngineEvent::ParticipantReconcile { seen_identities } => {
993+
self.reconcile_absent_participants(seen_identities.into_iter())
994+
}
978995
EngineEvent::MediaTrack { track, stream, transceiver } => {
979996
self.handle_media_track(track, stream, transceiver)
980997
}
@@ -1132,6 +1149,32 @@ impl RoomSession {
11321149
true
11331150
}
11341151

1152+
/// Synthesize the disconnection of every known remote participant missing
1153+
/// from `present`, for syncs where the server's participant list is
1154+
/// authoritative (post-resume reconcile, room move): a participant who
1155+
/// left while our signal link was down never got its DISCONNECTED update
1156+
/// delivered to us, and would otherwise stay in the room forever.
1157+
fn reconcile_absent_participants(
1158+
self: &Arc<Self>,
1159+
present: impl Iterator<Item = ParticipantIdentity>,
1160+
) {
1161+
let present: HashSet<ParticipantIdentity> = present.collect();
1162+
let missing: Vec<RemoteParticipant> = self
1163+
.remote_participants
1164+
.read()
1165+
.values()
1166+
.filter(|p| !present.contains(&p.identity()))
1167+
.cloned()
1168+
.collect();
1169+
for participant in missing {
1170+
log::info!(
1171+
"synthesizing disconnect for absent participant: {}",
1172+
participant.identity()
1173+
);
1174+
self.clone().handle_participant_disconnect(participant);
1175+
}
1176+
}
1177+
11351178
/// Update the participants inside a Room.
11361179
/// It'll create, update or remove a participant
11371180
/// It also update the participant tracks.
@@ -1536,6 +1579,11 @@ impl RoomSession {
15361579
participants: vec![Participant::Local(self.local_participant.clone())],
15371580
});
15381581
}
1582+
// Participants we knew from the old room that are absent from the
1583+
// moved-to room have left it.
1584+
self.reconcile_absent_participants(
1585+
moved.other_participants.iter().map(|pi| pi.identity.clone().into()),
1586+
);
15391587
self.handle_participant_update(moved.other_participants);
15401588
if let Some(room) = moved.room {
15411589
self.handle_room_update(room);

livekit/src/rtc_engine/mod.rs

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use livekit_datatrack::backend as dt;
1818
use livekit_protocol as proto;
1919
use livekit_runtime::JoinHandle;
2020
use parking_lot::{RwLock, RwLockReadGuard};
21-
use std::{borrow::Cow, fmt::Debug, sync::Arc, time::Duration};
21+
use std::{borrow::Cow, collections::HashSet, fmt::Debug, sync::Arc, time::Duration};
2222
use thiserror::Error;
2323
use tokio::sync::{
2424
mpsc, oneshot, Notify, RwLock as AsyncRwLock, RwLockReadGuard as AsyncRwLockReadGuard,
@@ -114,6 +114,12 @@ pub enum EngineEvent {
114114
ParticipantUpdate {
115115
updates: Vec<proto::ParticipantInfo>,
116116
},
117+
/// A signal resume fully recovered; any known participant whose identity
118+
/// is not in `seen_identities` left the room while the signal link was
119+
/// down and its disconnection must be synthesized.
120+
ParticipantReconcile {
121+
seen_identities: HashSet<ParticipantIdentity>,
122+
},
117123
MediaTrack {
118124
track: MediaStreamTrack,
119125
stream: MediaStream,
@@ -437,6 +443,13 @@ impl RtcEngine {
437443
.fail_transport_during_next_resume
438444
.store(true, std::sync::atomic::Ordering::Release);
439445
}
446+
447+
/// Test-only: drop incoming DISCONNECTED participant entries on the current
448+
/// session, simulating an SFU that fails to (re)deliver disconnect updates.
449+
#[cfg(feature = "__lk-e2e-test")]
450+
pub fn drop_disconnected_updates(&self, enabled: bool) {
451+
self.session().drop_disconnected_updates(enabled);
452+
}
440453
}
441454

442455
impl EngineInner {
@@ -1156,6 +1169,15 @@ impl EngineInner {
11561169
// has fully recovered, so deferred subscription updates / mutes / etc.
11571170
// should now reach the server. Mirrors `client.setReconnected()`.
11581171
session.signal_client().set_reconnected().await;
1172+
1173+
// Anyone who left while the signal link was down never got their
1174+
// DISCONNECTED update delivered to us; the room synthesizes those
1175+
// disconnects from the identities seen since the resume began. Sent
1176+
// from this task — the same producer that sends `Resumed` next — so
1177+
// they reach the application before `Reconnected`.
1178+
if let Some(seen_identities) = session.finish_resume() {
1179+
let _ = self.engine_tx.send(EngineEvent::ParticipantReconcile { seen_identities });
1180+
}
11591181
Ok(())
11601182
}
11611183
}

livekit/src/rtc_engine/rtc_session.rs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,21 @@ struct SessionInner {
401401

402402
participant_info: SessionParticipantInfo,
403403

404+
/// `Some` while a signal resume is in flight: accumulates the identity of
405+
/// every participant mentioned in `Update`s since the resume began. The
406+
/// server sends a full participant snapshot right after the
407+
/// `ReconnectResponse` but may interleave delayed/batched updates around
408+
/// it, so no single `Update` is identifiable as the snapshot — the union
409+
/// is what's guaranteed to cover everyone still in the room once the
410+
/// resume settles (see [`RtcSession::finish_resume`]).
411+
resume_seen_identities: Mutex<Option<HashSet<ParticipantIdentity>>>,
412+
413+
/// Test-only: drop incoming DISCONNECTED participant entries, simulating
414+
/// an SFU that fails to (re)deliver disconnect updates. Lets tests
415+
/// exercise the resume-time participant reconciliation deterministically.
416+
#[cfg(feature = "__lk-e2e-test")]
417+
drop_disconnected_updates: AtomicBool,
418+
404419
dc_emitter: mpsc::UnboundedSender<DataChannelEvent>,
405420

406421
// Keep a strong reference to the subscriber datachannels,
@@ -620,6 +635,9 @@ impl RtcSession {
620635
next_packet_sequence: 1.into(),
621636
packet_rx_state: Mutex::new(TtlMap::new(RELIABLE_RECEIVED_STATE_TTL)),
622637
participant_info,
638+
resume_seen_identities: Mutex::new(None),
639+
#[cfg(feature = "__lk-e2e-test")]
640+
drop_disconnected_updates: Default::default(),
623641
dc_emitter,
624642
sub_lossy_dc: Mutex::new(None),
625643
sub_reliable_dc: Mutex::new(None),
@@ -809,6 +827,24 @@ impl RtcSession {
809827
self.inner.restart_publisher().await
810828
}
811829

830+
/// Ends the resume-time accumulation started by [`Self::restart`],
831+
/// returning the participant identities seen since. The post-resume
832+
/// snapshot arrived several round trips before the PeerConnections
833+
/// finished reconnecting, so this is a superset of the room's current
834+
/// participants: any known participant absent from it left while the
835+
/// signal link was down and its disconnection must be synthesized.
836+
/// Returns `None` if no resume was in flight.
837+
pub fn finish_resume(&self) -> Option<HashSet<ParticipantIdentity>> {
838+
self.inner.resume_seen_identities.lock().take()
839+
}
840+
841+
/// Test-only: drop incoming DISCONNECTED participant entries, simulating
842+
/// an SFU that fails to (re)deliver disconnect updates.
843+
#[cfg(feature = "__lk-e2e-test")]
844+
pub fn drop_disconnected_updates(&self, enabled: bool) {
845+
self.inner.drop_disconnected_updates.store(enabled, Ordering::Release);
846+
}
847+
812848
pub async fn wait_pc_connection(&self) -> EngineResult<()> {
813849
self.inner.wait_pc_connection().await
814850
}
@@ -1387,13 +1423,26 @@ impl SessionInner {
13871423
);
13881424
}
13891425
proto::signal_response::Message::Update(mut update) => {
1426+
#[cfg(feature = "__lk-e2e-test")]
1427+
// injecting faulty behaviour during a signal disconnect to ensure we can mimic
1428+
// losing/missing participant disconnect events after a resume
1429+
// for test_resume_synthesizes_disconnect_for_participant_that_left
1430+
if self.drop_disconnected_updates.load(Ordering::Acquire) {
1431+
update.participants.retain(|pi| {
1432+
pi.state != proto::participant_info::State::Disconnected as i32
1433+
});
1434+
}
1435+
13901436
let local_participant_identity = self.participant_info.identity.as_str().into();
13911437
if let Ok(event) = dt::remote::event_from_participant_update(
13921438
&mut update,
13931439
local_participant_identity,
13941440
) {
13951441
_ = self.emitter.send(SessionEvent::RemoteDataTrackInput(event.into()));
13961442
}
1443+
if let Some(seen) = self.resume_seen_identities.lock().as_mut() {
1444+
seen.extend(update.participants.iter().map(|pi| pi.identity.clone().into()));
1445+
}
13971446
let _ = self
13981447
.emitter
13991448
.send(SessionEvent::ParticipantUpdate { updates: update.participants });
@@ -2140,6 +2189,10 @@ impl SessionInner {
21402189
/// This reconnection if more seemless compared to the full reconnection implemented in
21412190
/// ['RTCEngine']
21422191
async fn restart(&self) -> EngineResult<proto::ReconnectResponse> {
2192+
// Start accumulating before the signal client reconnects: once
2193+
// `restart` returns, the resumed stream immediately delivers events
2194+
// (including the post-resume participant snapshot) on a concurrent task.
2195+
*self.resume_seen_identities.lock() = Some(HashSet::new());
21432196
let reconnect_response = self.signal_client.restart().await?;
21442197
log::debug!("received reconnect response: {:?}", reconnect_response);
21452198

0 commit comments

Comments
 (0)