Fix crash when publishing disposed tracks (#758)

* Fix crash when publishing disposed tracks

* Fix tests

Author: davidliu

.changeset/gold-cats-knock.md

@@ -0,0 +1,5 @@
+---
+"client-sdk-android": patch
+---
+
+Fix crash when publishing disposed tracks

livekit-android-sdk/src/main/java/io/livekit/android/room/participant/LocalParticipant.kt

@@ -432,6 +432,7 @@ internal constructor(
      *
      * @param track The track to publish.
      * @param options The publish options to use, or [Room.audioTrackPublishDefaults] if none is passed.
+     * @return true if the track published successfully
      */
     suspend fun publishAudioTrack(
         track: LocalAudioTrack,
@@ -441,25 +442,35 @@ internal constructor(
         ).copy(preconnect = defaultsManager.isPrerecording),
         publishListener: PublishListener? = null,
     ): Boolean {
+        if (track.isDisposed) {
+            LKLog.w { "Attempting to publish a disposed track, ignoring." }
+            return false
+        }
+
         val encodings = listOf(
             RtpParameters.Encoding(null, true, null).apply {
                 if (options.audioBitrate != null && options.audioBitrate > 0) {
                     maxBitrateBps = options.audioBitrate
                 }
             },
         )
-        val publication = publishTrackImpl(
-            track = track,
-            options = options,
-            requestConfig = {
-                disableDtx = !options.dtx
-                disableRed = !options.red
-                addAllAudioFeatures(options.getFeaturesList())
-                source = options.source?.toProto() ?: LivekitModels.TrackSource.MICROPHONE
-            },
-            encodings = encodings,
-            publishListener = publishListener,
-        )
+        var publication: LocalTrackPublication? = null
+        try {
+            publication = publishTrackImpl(
+                track = track,
+                options = options,
+                requestConfig = {
+                    disableDtx = !options.dtx
+                    disableRed = !options.red
+                    addAllAudioFeatures(options.getFeaturesList())
+                    source = options.source?.toProto() ?: LivekitModels.TrackSource.MICROPHONE
+                },
+                encodings = encodings,
+                publishListener = publishListener,
+            )
+        } catch (e: TrackException.PublishException) {
+            LKLog.e(e) { "Error thrown when publishing track:" }
+        }
 
         if (publication != null) {
             val job = scope.launch {
@@ -478,6 +489,7 @@ internal constructor(
      *
      * @param track The track to publish.
      * @param options The publish options to use, or [Room.videoTrackPublishDefaults] if none is passed.
+     * @return true if the track published successfully
      */
     suspend fun publishVideoTrack(
         track: LocalVideoTrack,
@@ -489,6 +501,17 @@ internal constructor(
     ): Boolean {
         @Suppress("NAME_SHADOWING") var options = options
 
+        if (track.isDisposed) {
+            LKLog.w { "Attempting to publish a disposed track, ignoring." }
+            return false
+        }
+
+        val rtcTrackId = track.withRTCTrack(null) { id() }
+        if (rtcTrackId == null) {
+            LKLog.w { "Attempting to publish a disposed track, ignoring." }
+            return false
+        }
+
         synchronized(enabledPublishVideoCodecs) {
             if (enabledPublishVideoCodecs.isNotEmpty()) {
                 if (enabledPublishVideoCodecs.none { allowedCodec -> allowedCodec.mime.mimeTypeToVideoCodec() == options.videoCodec }) {
@@ -522,40 +545,47 @@ internal constructor(
         val videoLayers =
             EncodingUtils.videoLayersFromEncodings(track.dimensions.width, track.dimensions.height, encodings, isSVC)
 
-        return publishTrackImpl(
-            track = track,
-            options = options,
-            requestConfig = {
-                width = track.dimensions.width
-                height = track.dimensions.height
-                source = options.source?.toProto() ?: if (track.options.isScreencast) {
-                    LivekitModels.TrackSource.SCREEN_SHARE
-                } else {
-                    LivekitModels.TrackSource.CAMERA
-                }
-                addAllLayers(videoLayers)
+        var publication: LocalTrackPublication? = null
+        try {
+            publication = publishTrackImpl(
+                track = track,
+                options = options,
+                requestConfig = {
+                    width = track.dimensions.width
+                    height = track.dimensions.height
+                    source = options.source?.toProto() ?: if (track.options.isScreencast) {
+                        LivekitModels.TrackSource.SCREEN_SHARE
+                    } else {
+                        LivekitModels.TrackSource.CAMERA
+                    }
+                    addAllLayers(videoLayers)
 
-                addSimulcastCodecs(
-                    with(SimulcastCodec.newBuilder()) {
-                        codec = options.videoCodec
-                        cid = track.rtcTrack.id()
-                        build()
-                    },
-                )
-                // set up backup codec
-                if (options.backupCodec?.codec != null && options.videoCodec != options.backupCodec?.codec) {
                     addSimulcastCodecs(
                         with(SimulcastCodec.newBuilder()) {
-                            codec = options.backupCodec!!.codec
-                            cid = ""
+                            codec = options.videoCodec
+                            cid = rtcTrackId
                             build()
                         },
                     )
-                }
-            },
-            encodings = encodings,
-            publishListener = publishListener,
-        ) != null
+                    // set up backup codec
+                    if (options.backupCodec?.codec != null && options.videoCodec != options.backupCodec?.codec) {
+                        addSimulcastCodecs(
+                            with(SimulcastCodec.newBuilder()) {
+                                codec = options.backupCodec!!.codec
+                                cid = ""
+                                build()
+                            },
+                        )
+                    }
+                },
+                encodings = encodings,
+                publishListener = publishListener,
+            )
+        } catch (e: TrackException.PublishException) {
+            LKLog.e(e) { "Error thrown when publishing track:" }
+        }
+
+        return publication != null
     }
 
     private fun hasPermissionsToPublish(source: Track.Source): Boolean {
@@ -581,13 +611,19 @@ internal constructor(
      * @throws TrackException.PublishException thrown when the publish fails. see [TrackException.PublishException.message] for details.
      * @return true if the track publish was successful.
      */
+    @Throws(TrackException.PublishException::class)
     private suspend fun publishTrackImpl(
         track: Track,
         options: TrackPublishOptions,
         requestConfig: AddTrackRequest.Builder.() -> Unit,
         encodings: List<RtpParameters.Encoding> = emptyList(),
         publishListener: PublishListener? = null,
     ): LocalTrackPublication? {
+        if (track.isDisposed) {
+            LKLog.w { "Attempting to publish a disposed track, ignoring." }
+            return null
+        }
+
         fun onPublishFailure(e: TrackException.PublishException, triggerEvent: Boolean = true) {
             publishListener?.onPublishFailure(e)
             if (triggerEvent) {
@@ -1164,6 +1200,7 @@ internal constructor(
                     continuation.cancel(RpcError.BuiltinRpcError.RESPONSE_TIMEOUT.create())
                 }
             }
+            responseTimeoutJob // workaround for lint marking this unused. used in cleanup()
 
             pendingResponses[requestId] = PendingRpcResponse(
                 participantIdentity = destinationIdentity,

livekit-android-sdk/src/main/java/io/livekit/android/room/track/Track.kt

@@ -191,6 +191,9 @@ abstract class Track(
         }
     }
 
+    /**
+     * Ensures the track is valid before attempting to run [action].
+     */
     @OptIn(ExperimentalContracts::class)
     internal inline fun <T> withRTCTrack(crossinline action: MediaStreamTrack.() -> T) {
         contract { callsInPlace(action, InvocationKind.AT_MOST_ONCE) }

livekit-android-test/src/main/java/io/livekit/android/test/mock/MockRTCThreadToken.kt

@@ -20,5 +20,5 @@ import io.livekit.android.webrtc.peerconnection.RTCThreadToken
 
 class MockRTCThreadToken : RTCThreadToken {
     override val isDisposed: Boolean
-        get() = true
+        get() = false
 }

livekit-android-test/src/main/java/io/livekit/android/test/mock/dagger/TestRTCModule.kt

@@ -90,7 +90,6 @@ object TestRTCModule {
         appContext: Context,
     ): PeerConnectionFactory {
         WebRTCInitializer.initialize(appContext)
-
         return MockPeerConnectionFactory()
     }
 

livekit-android-test/src/test/java/io/livekit/android/room/participant/LocalParticipantMockE2ETest.kt

@@ -789,7 +789,7 @@ class LocalParticipantMockE2ETest : MockE2ETest() {
     }
 
     @Test
-    fun lackOfPublishPermissionCausesException() = runTest {
+    fun lackOfPublishPermissionReturnsFalse() = runTest {
         val noCanPublishJoin = with(TestData.JOIN.toBuilder()) {
             join = with(join.toBuilder()) {
                 participant = with(participant.toBuilder()) {
@@ -805,14 +805,7 @@ class LocalParticipantMockE2ETest : MockE2ETest() {
         }
         connect(noCanPublishJoin)
 
-        var didThrow = false
-        try {
-            room.localParticipant.publishVideoTrack(createLocalTrack())
-        } catch (e: TrackException.PublishException) {
-            didThrow = true
-        }
-
-        assertTrue(didThrow)
+        assertFalse(room.localParticipant.publishVideoTrack(createLocalTrack()))
     }
 
     @Test