Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tall-birds-rest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@livekit/rtc-node': patch
---

Deduplicate getSid() listeners to prevent event listener leak on concurrent calls
38 changes: 27 additions & 11 deletions packages/livekit-rtc/src/room.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,11 @@ export class Room extends (EventEmitter as new () => TypedEmitter<RoomCallbacks>
return this._serverUrl;
}

// Shared promise for concurrent getSid() callers. Without this, each call
// registers its own RoomSidChanged + Disconnected listeners, and if many
// calls race only one of each pair is cleaned up — leaking the rest.
private sidPromise?: Promise<string>;

/**
* Gets the room's server ID. This ID is assigned by the LiveKit server
* and is unique for each room session.
Expand All @@ -144,19 +149,26 @@ export class Room extends (EventEmitter as new () => TypedEmitter<RoomCallbacks>
if (this.info?.sid && this.info.sid !== '') {
return this.info.sid;
}
return new Promise((resolve, reject) => {
const handleRoomUpdate = (sid: string) => {
if (sid !== '') {
if (!this.sidPromise) {
this.sidPromise = new Promise<string>((resolve, reject) => {
const handleDisconnect = () => {
this.off(RoomEvent.RoomSidChanged, handleRoomUpdate);
resolve(sid);
}
};
this.on(RoomEvent.RoomSidChanged, handleRoomUpdate);
this.once(RoomEvent.Disconnected, () => {
this.off(RoomEvent.RoomSidChanged, handleRoomUpdate);
reject('Room disconnected before room server id was available');
this.sidPromise = undefined;
reject('Room disconnected before room server id was available');
};
const handleRoomUpdate = (sid: string) => {
if (sid !== '') {
this.off(RoomEvent.RoomSidChanged, handleRoomUpdate);
this.off(RoomEvent.Disconnected as any, handleDisconnect);
this.sidPromise = undefined;
resolve(sid);
}
};
this.on(RoomEvent.RoomSidChanged, handleRoomUpdate);
this.once(RoomEvent.Disconnected, handleDisconnect);
});
});
}
return this.sidPromise;
}

get numParticipants(): number {
Expand Down Expand Up @@ -283,6 +295,10 @@ export class Room extends (EventEmitter as new () => TypedEmitter<RoomCallbacks>
return ev.message.case == 'disconnect' && ev.message.value.asyncId == res.asyncId;
});

// Clear sidPromise before removing listeners so that a reconnect
// doesn't return a stale, permanently-pending promise.
this.sidPromise = undefined;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should already be accounted for if you don't remove the handleDisconnect once listener above?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The once-listener depends on the 'disconnected' FFI event arriving and being processed before the 'disconnect' callback resolves. Since we can't guarantee that ordering, and if the callback resolves first, removeAllListeners() wipes the once-listener before it fires, the explicit cleanup here is needed as a safety measure to prevent a stale sidPromise across reconnects.


FfiClient.instance.removeListener(FfiClientEvent.FfiEvent, this.onFfiEvent);
this.removeAllListeners();
}
Expand Down
20 changes: 20 additions & 0 deletions packages/livekit-rtc/src/tests/e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -514,4 +514,24 @@ describeE2E('livekit-rtc e2e', () => {
},
testTimeoutMs * 2,
);

it(
'concurrent getSid() calls share a single listener and resolve consistently',
async () => {
const { rooms } = await connectTestRooms(1);
const room = rooms[0]!;

// Fire multiple concurrent getSid() calls — they should all resolve
// to the same SID without leaking event listeners.
const results = await Promise.all([room.getSid(), room.getSid(), room.getSid()]);

// All calls should return the same non-empty SID
expect(results[0]).toBeTruthy();
expect(results[1]).toBe(results[0]);
expect(results[2]).toBe(results[0]);

await room.disconnect();
},
testTimeoutMs,
);
});
Loading