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
13 changes: 13 additions & 0 deletions .changeset/bound-subscriber-ice-restart-window.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
'livekit-client': patch
---

Fix the subscriber silently buffering remote ICE candidates after a reconnect

`triggerIceRestart` put the subscriber into `restartingIce` on every reconnect, but only
`setRemoteDescription` clears that — and the server re-offers the subscriber only when the
reconnect moved the participant to a different node. After an ordinary signal-only resume no
offer arrives, so the flag stayed set for the lifetime of the transport and every subsequent
remote candidate was queued instead of applied, leaving the subscriber unable to adopt any new
network path the server proposed. The subscriber no longer enters that state: the server does
not send candidates ahead of the offer that introduces them, so queueing them gains nothing.
35 changes: 35 additions & 0 deletions src/room/PCTransportManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,3 +279,38 @@ describe('PCTransportManager.negotiate', () => {
await expect(p).resolves.toBeUndefined();
});
});

describe('PCTransportManager.triggerIceRestart', () => {
let originalRTCPeerConnection: unknown;

beforeEach(() => {
originalRTCPeerConnection = (globalThis as unknown as { RTCPeerConnection?: unknown })
.RTCPeerConnection;
(globalThis as unknown as { RTCPeerConnection: unknown }).RTCPeerConnection = StubPC;
});

afterEach(() => {
(globalThis as unknown as { RTCPeerConnection: unknown }).RTCPeerConnection =
originalRTCPeerConnection;
});

/**
* The subscriber must keep applying remote candidates across a reconnect.
*
* Putting it into `restartingIce` would queue them until a new remote description arrives —
* but the server only re-offers the subscriber when the reconnect moved us to another node,
* so on an ordinary signal-only resume nothing would ever flush that queue, and the
* transport would stop adopting new network paths for the rest of the session. The server
* also never sends candidates ahead of the offer that introduces them, so queueing buys
* nothing in exchange.
*/
it('does not stop the subscriber applying remote candidates', async () => {
const manager = new PCTransportManager('subscriber-primary', {});
const publisher = new FakePublisher();
(manager as unknown as { publisher: FakePublisher }).publisher = publisher;

await manager.triggerIceRestart();

expect(manager.subscriber?.restartingIce).toBe(false);
});
});
16 changes: 12 additions & 4 deletions src/room/PCTransportManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,20 @@ export class PCTransportManager {
this.updateState();
}

/**
* Restarts ICE on the transports that need it. Only the publisher: the server restarts the
* subscriber's ICE itself and follows with a fresh offer.
*
* The subscriber deliberately does NOT enter `restartingIce` here. Queueing its remote
* candidates would guard against candidates for a new generation arriving before the offer
* that introduces it, but the server does not send them in that order -- on a same-node
* resume it buffers them until the offer has gone out, and on a reconnect that lands on
* another node it withholds subscriber candidates until immediately before creating the
* offer. Setting the flag only risks withholding candidates during the window that decides
* whether the reconnect succeeded.
*/
async triggerIceRestart() {
this.iceLog.warn('triggering ICE restart');
if (this.subscriber) {

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.

What this fixes: after any reconnect, the subscriber silently stops applying
remote ICE candidates for the rest of the session.

triggerIceRestart() set subscriber.restartingIce = true. That flag tells
addIceCandidate to park candidates in pendingCandidates instead of applying
them, and the only code that clears it is setRemoteDescription — which runs on
the subscriber only when the server re-offers it, i.e. only when the reconnect
moved the participant to a different node. After an ordinary signal-only resume
no offer arrives, so the flag stays set for the transport's lifetime and every
later candidate is parked. The subscriber can no longer adopt a new network path
the server proposes (route change, NAT rebind, switch to relay), so media can
stall until some unrelated negotiation happens to flush the queue.

The fix: stop setting it. The queueing existed to stop candidates for a new
ICE generation being applied against the old remote description, but the server
never sends them in that order — a same-node resume buffers its candidates until
after the offer is sent, and a reconnect landing on another node drops subscriber
candidates until immediately before the offer is created. So the flag had no
upside and one permanent downside.

After this change nothing can set subscriber.restartingIce at all: the only
= true is in createAndSendOffer, reachable only for the publisher. The
publisher's own use is sound and untouched — it's set as an offer is sent, and
the answer clears it.

The line worth making sure a reviewer sees is the asymmetry: the publisher sets the flag alongside an offer that will certainly be answered, so it always clears; the subscriber's was set on the expectation of an offer that usually never comes. That's the whole bug in one sentence.

this.subscriber.restartingIce = true;
}
// only restart publisher if it's needed
if (this.needsPublisher) {
await this.createAndSendPublisherOffer({ iceRestart: true });
}
Expand Down
Loading