Fixed setTrackEnabled orphaning track resources when cancelled before publish completes

A cancellation landing in startForegroundService or publishVideoTrack
unwound out of setTrackEnabled without any cleanup, abandoning the newly
created screencast track and, when the bind had already succeeded,
leaving ScreenCaptureService bound for the lifetime of the context. Each
enable path tears the unpublished track down in a finally block, so
cancellation and publish failures release capture, track resources, and
the service binding, and invoke the onStop callback exactly once, even
when the platform ended the projection while publishing was in flight.

A projection stop already dispatched on its callback thread can reach
the track after that cleanup disposed it, where ScreenCapturerAndroid
throws from stopCapture. LocalScreencastVideoTrack.stop() tolerates
losing that race instead of crashing the callback thread.

publishTrackImpl negotiates the sender transceiver concurrently with the
add track request, so a failed or cancelled request could leave a
negotiated transceiver holding its sender and SDP m-section with no
publication to unpublish. The rollback targets exactly the sender this
attempt created on the transport that created it, so it cannot detach a
concurrent publish of the same track, and the handle is recorded
non-cancellably so a cancellation cannot discard the RTC-thread result.
Sender handles retain the signal session state that owned their
transceiver. Aborting pending publishes and starting a reconnect replace
that state before cleanup can run, so a delayed continuation cannot
mutate the same publisher after a soft reconnect. The final eligibility
check runs on the RTC thread immediately before mutation. Stopping the
transceiver and releasing its m-section is reserved for video, matching
unpublishTrack.

LocalScreencastVideoTrack also never released its SurfaceTextureHelper,
so its capture thread outlived dispose() even on the normal teardown
path. The helper is registered with the track's CloseableManager,
matching LocalVideoTrack.

Fixes #985

Author: adrian-niculescu

.changeset/set-track-enabled-cancellation-cleanup.md

@@ -0,0 +1,5 @@
+---
+"client-sdk-android": patch
+---
+
+Fixed setTrackEnabled leaking track resources when cancelled before the track is published, including the sender negotiated for a failed publish, and LocalScreencastVideoTrack leaking its SurfaceTextureHelper on dispose.

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

@@ -133,6 +133,7 @@ internal constructor(
         }
         when (newVal) {
             ConnectionState.CONNECTED -> {
+                signalSessionState = SignalSessionState(ended = false)
                 if (oldVal == ConnectionState.DISCONNECTED || oldVal == ConnectionState.CONNECTING) {
                     LKLog.d { "primary ICE connected" }
                     listener?.onEngineConnected()
@@ -158,8 +159,17 @@ internal constructor(
 
     @Volatile
     internal var reconnectType: ReconnectType = ReconnectType.DEFAULT
+
+    @Volatile
     private var reconnectingJob: Job? = null
 
+    /**
+     * Replaced at each signal session boundary. Sender handles retain the state that owned their
+     * transceiver, so cleanup cannot mutate a publisher after that session ends.
+     */
+    @Volatile
+    private var signalSessionState = SignalSessionState(ended = true)
+
     @Volatile
     private var fullReconnectOnNext = false
 
@@ -424,10 +434,13 @@ internal constructor(
     internal suspend fun createSenderTransceiver(
         rtcTrack: MediaStreamTrack,
         transInit: RtpTransceiverInit,
-    ): RtpTransceiver? {
-        return publisher?.withPeerConnection {
+    ): SenderTransceiverHandle? {
+        val sessionState = signalSessionState
+        val publisher = publisher ?: return null
+        val transceiver = publisher.withPeerConnection {
             addTransceiver(rtcTrack, transInit)
-        }
+        } ?: return null
+        return SenderTransceiverHandle(publisher, transceiver, sessionState)
     }
 
     fun updateSubscriptionPermissions(
@@ -505,6 +518,9 @@ internal constructor(
     }
 
     private fun abortPendingPublishTracks() {
+        // Ordered ahead of the resumes: each one unwinds a publish into cleanup that reads this,
+        // and on some paths the reconnect is only triggered later, by the socket closing.
+        signalSessionState = SignalSessionState(ended = true)
         synchronized(pendingTrackResolvers) {
             pendingTrackResolvers.values.forEach {
                 it.resumeWithException(TrackException.PublishException("pending track aborted"))
@@ -535,6 +551,7 @@ internal constructor(
         }
         val forceFullReconnect = fullReconnectOnNext
         fullReconnectOnNext = false
+        signalSessionState = SignalSessionState(ended = true)
         val job = coroutineScope.launch {
             var hasResumedOnce = false
             var hasReconnectedOnce = false
@@ -1515,15 +1532,64 @@ internal constructor(
         }
     }
 
+    /**
+     * Detaches a sender transceiver whose publish attempt produced no publication.
+     *
+     * A publish fails most often because the signal connection died, and that same failure
+     * starts a reconnect which renegotiates this peer connection. Mutating its senders and
+     * transceivers alongside that negotiation races it, so the rollback is skipped unless the
+     * publisher is connected and idle. The sender retains its signal session state so an aborted
+     * publish cannot mutate the same publisher after a soft reconnect.
+     *
+     * @param stopTransceiver releases the transceiver and its SDP m-section for reuse. Reserved
+     * for video, matching the teardown an unpublish performs.
+     * @return whether the sender was detached, so the track can drop its reference to it.
+     */
+    internal fun rollbackSenderTransceiver(
+        senderTransceiver: SenderTransceiverHandle,
+        stopTransceiver: Boolean,
+    ): Boolean {
+        val publisher = senderTransceiver.publisher
+        return runBlocking {
+            publisher.withPeerConnection {
+                val sessionState = signalSessionState
+                val publisherIsIdle = this@RTCEngine.publisher === publisher &&
+                    connectionState == ConnectionState.CONNECTED &&
+                    reconnectingJob?.isActive != true &&
+                    senderTransceiver.signalSessionState === sessionState &&
+                    !sessionState.ended
+                if (!publisherIsIdle) {
+                    return@withPeerConnection false
+                }
+                val transceiver = senderTransceiver.transceiver
+                removeTrack(transceiver.sender)
+                if (stopTransceiver && !transceiver.isStopped) {
+                    transceiver.stopInternal()
+                }
+                true
+            } ?: false
+        }
+    }
+
     @VisibleForTesting
     fun getPublisherPeerConnection() =
         publisher!!.peerConnection
 
     @VisibleForTesting
     fun getSubscriberPeerConnection() =
         subscriber!!.peerConnection
+
+    private class SignalSessionState(
+        val ended: Boolean,
+    )
 }
 
+internal class SenderTransceiverHandle(
+    internal val publisher: PeerConnectionTransport,
+    internal val transceiver: RtpTransceiver,
+    internal val signalSessionState: Any,
+)
+
 /**
  * @suppress
  */