diff --git a/.changeset/ignore-duplicate-answer.md b/.changeset/ignore-duplicate-answer.md new file mode 100644 index 000000000..645ba50b6 --- /dev/null +++ b/.changeset/ignore-duplicate-answer.md @@ -0,0 +1,9 @@ +--- +"client-sdk-android": patch +--- + +Ignore a duplicated server answer. `PeerConnectionTransport.setRemoteDescription` rejected answers carrying an *older* offer id, but a second copy of the answer for the *current* offer passed straight through to `setRemoteDescription`. A proxy duplicating the signal message, or a server retry, therefore applied the same answer twice — the second time with no local offer left to answer, which is an invalid transition and left the transport reporting an error on a renegotiation that had in fact succeeded. + +The transport now tracks the offer whose answer was applied, and the offer whose answer is currently being applied, and drops a repeat of either. A retry after a genuinely failed apply is still accepted, and the legacy unnumbered-answer path falls back to the signaling state. + +This matches the JavaScript SDK (livekit/client-sdk-js#2114). diff --git a/livekit-android-sdk/src/main/java/io/livekit/android/room/PeerConnectionTransport.kt b/livekit-android-sdk/src/main/java/io/livekit/android/room/PeerConnectionTransport.kt index 9a4c9c6f9..1c3e7c49f 100644 --- a/livekit-android-sdk/src/main/java/io/livekit/android/room/PeerConnectionTransport.kt +++ b/livekit-android-sdk/src/main/java/io/livekit/android/room/PeerConnectionTransport.kt @@ -115,6 +115,13 @@ constructor( private val latestOfferId = AtomicInteger(0) + /** offerId of the last answer successfully applied, or 0 if none. */ + private val answeredOfferId = AtomicInteger(0) + + /** offerId (0 if unnumbered) of the answer being applied right now, or null if none. */ + @Volatile + private var answerInFlight: Int? = null + interface Listener { fun onOffer(sd: SessionDescription, offerId: Int) } @@ -141,15 +148,36 @@ constructor( if (sd.type == SessionDescription.Type.ANSWER && currentOfferId > 0 && offerId > 0 && currentOfferId > offerId) { return@launchRTCIfNotClosed Either.Right("Old offer, ignoring. Expected: $currentOfferId, actual: $offerId") } - val result = peerConnection.setRemoteDescription(sd) - if (result is Either.Left) { - pendingCandidates.forEach { pending -> - peerConnection.addIceCandidate(pending) + if (sd.type == SessionDescription.Type.ANSWER) { + if (isDuplicateAnswer(offerId, answerInFlight, answeredOfferId.get(), peerConnection.signalingState())) { + // a repeat of an answer already applied or being applied (e.g. duplicated by a + // proxy): there is no local offer left to answer + LKLog.w { "ignoring duplicate answer, offerId: $offerId" } + return@launchRTCIfNotClosed Either.Right("Duplicate answer, ignoring. offerId: $offerId") + } + // claimed before any suspension, so a copy arriving mid-apply is caught too + answerInFlight = offerId + } + try { + val result = peerConnection.setRemoteDescription(sd) + if (result is Either.Left) { + // only a successfully applied answer makes later copies duplicates + if (sd.type == SessionDescription.Type.ANSWER && offerId > 0) { + answeredOfferId.set(offerId) + } + pendingCandidates.forEach { pending -> + peerConnection.addIceCandidate(pending) + } + pendingCandidates.clear() + restartingIce = false + } + return@launchRTCIfNotClosed result + } finally { + // released however the apply ends, so a genuine retry is still possible + if (sd.type == SessionDescription.Type.ANSWER) { + answerInFlight = null } - pendingCandidates.clear() - restartingIce = false } - return@launchRTCIfNotClosed result } ?: Either.Right("PCT is closed.") if (this.renegotiate) { @@ -740,3 +768,21 @@ internal sealed class TrackBitrateInfoKey { data class Cid(val value: String) : TrackBitrateInfoKey() data class Transceiver(val value: RtpTransceiver) : TrackBitrateInfoKey() } + +/** + * Whether an incoming answer repeats one already applied or being applied. + * + * Any answer for the offer whose answer is in flight is a duplicate, numbered or not. With an + * [offerId] (current servers) an answer is also a duplicate if that offer's answer was already + * applied. Without one, fall back to the signaling state: in `STABLE` no local offer is pending. + */ +internal fun isDuplicateAnswer( + offerId: Int, + answerInFlight: Int?, + answeredOfferId: Int, + signalingState: PeerConnection.SignalingState?, +): Boolean { + if (answerInFlight != null && answerInFlight == offerId) return true + if (offerId > 0) return offerId == answeredOfferId + return signalingState == PeerConnection.SignalingState.STABLE +} diff --git a/livekit-android-test/src/test/java/io/livekit/android/room/IsDuplicateAnswerTest.kt b/livekit-android-test/src/test/java/io/livekit/android/room/IsDuplicateAnswerTest.kt new file mode 100644 index 000000000..351bd4acc --- /dev/null +++ b/livekit-android-test/src/test/java/io/livekit/android/room/IsDuplicateAnswerTest.kt @@ -0,0 +1,66 @@ +/* + * Copyright 2026 LiveKit, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.livekit.android.room + +import livekit.org.webrtc.PeerConnection.SignalingState +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class IsDuplicateAnswerTest { + + @Test + fun `drops an answer while the same offer is still being answered`() { + assertTrue(isDuplicateAnswer(3, 3, 0, SignalingState.HAVE_LOCAL_OFFER)) + } + + @Test + fun `drops a repeat of an unnumbered answer while the first is still applying`() { + assertTrue(isDuplicateAnswer(0, 0, 0, SignalingState.HAVE_LOCAL_OFFER)) + } + + @Test + fun `drops an answer for an offer whose answer was already applied`() { + assertTrue(isDuplicateAnswer(3, null, 3, SignalingState.STABLE)) + } + + @Test + fun `accepts a retry after a failed apply`() { + // Nothing in flight and nothing applied for this offer, so the answer is still wanted. + assertFalse(isDuplicateAnswer(3, null, 2, SignalingState.HAVE_LOCAL_OFFER)) + } + + @Test + fun `accepts the answer for a new offer`() { + assertFalse(isDuplicateAnswer(4, null, 3, SignalingState.HAVE_LOCAL_OFFER)) + } + + @Test + fun `without an offerId drops an answer in stable`() { + assertTrue(isDuplicateAnswer(0, null, 0, SignalingState.STABLE)) + } + + @Test + fun `without an offerId accepts an answer while a local offer is pending`() { + assertFalse(isDuplicateAnswer(0, null, 0, SignalingState.HAVE_LOCAL_OFFER)) + } + + @Test + fun `an answer in flight for a different offer does not mask a new one`() { + assertFalse(isDuplicateAnswer(4, 3, 0, SignalingState.HAVE_LOCAL_OFFER)) + } +} diff --git a/livekit-android-test/src/test/java/io/livekit/android/room/RTCEngineMockE2ETest.kt b/livekit-android-test/src/test/java/io/livekit/android/room/RTCEngineMockE2ETest.kt index 6225fb1ec..f681e96f9 100644 --- a/livekit-android-test/src/test/java/io/livekit/android/room/RTCEngineMockE2ETest.kt +++ b/livekit-android-test/src/test/java/io/livekit/android/room/RTCEngineMockE2ETest.kt @@ -257,6 +257,63 @@ class RTCEngineMockE2ETest : MockE2ETest() { } } + @Test + fun duplicateAnswerForTheSameOfferIsIgnored() = runTest { + connect() + + val ws = wsFactory.ws + var answeredOfferId = 0 + val handler: SignalRequestHandler = { request -> + if (request.hasOffer()) { + answeredOfferId = request.offer.id + val answer = with(LivekitRtc.SignalResponse.newBuilder()) { + answer = with(LivekitRtc.SessionDescription.newBuilder()) { + sdp = "remote_answer" + type = "answer" + id = request.offer.id + build() + } + build() + } + wsFactory.receiveMessage(answer) + true + } + false + } + wsFactory.registerSignalRequestHandler(handler) + + val publisher = rtcEngine.getPublisherPeerConnection() as MockPeerConnection + ws.clearRequests() + publisher.observer?.onRenegotiationNeeded() + advanceUntilIdle() + + assertEquals(PeerConnection.SignalingState.STABLE, publisher.signalingState()) + assertEquals("remote_answer", publisher.remoteDesc?.description) + wsFactory.unregisterSignalRequestHandler(handler) + + // A proxy (or a server retry) can deliver a second copy of the answer that was + // already applied. There is no local offer left to answer, so applying it again + // is invalid. The payload differs only so the test can tell whether it was applied. + val duplicate = with(LivekitRtc.SignalResponse.newBuilder()) { + answer = with(LivekitRtc.SessionDescription.newBuilder()) { + sdp = "remote_answer_duplicate" + type = "answer" + id = answeredOfferId + build() + } + build() + } + wsFactory.receiveMessage(duplicate) + advanceUntilIdle() + + assertEquals(PeerConnection.SignalingState.STABLE, publisher.signalingState()) + assertEquals( + "duplicate answer was applied to the peer connection", + "remote_answer", + publisher.remoteDesc?.description, + ) + } + @Test fun offerIdMismatchIsIgnored() = runTest { connect()