Fixed ScreenCaptureService staying bound when screen share setup is cancelled

.changeset/screen-capture-connection-bind-leak.md

@@ -0,0 +1,7 @@
+---
+"client-sdk-android": patch
+---
+
+Fix `ScreenCaptureService` staying bound when screen share setup is cancelled. `ScreenCaptureConnection` recorded a binding only once `onServiceConnected` arrived, so a coroutine cancelled before `connect()` returned left the `ServiceConnection` registered, and `BIND_AUTO_CREATE` kept the service alive for the lifetime of the context. `LocalParticipant.setScreenShareEnabled` awaits the bind internally and abandons the track it just created if cancelled, so nothing reached `stop()` on that path. A cancelled connect now releases the binding itself once no caller is left waiting on it, and `stop()` unbinds on the same wider condition. Callers stay tracked until `connect()` actually returns, so a cancellation landing after the service connected but before the caller resumed releases the binding too. This also covers the documented case where `bindService` leaves a connection registered while reporting failure or throwing.
+
+Two paths that could leave `connect()` suspended forever are fixed as well. `stop()` racing a connect no longer strands the caller, since requesting the bind and registering the waiter now happen under one lock, and a failed `bindService` no longer leaves the state claiming a bind is in flight for the next caller to wait on.

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2023-2025 LiveKit, Inc.
+ * Copyright 2023-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.
@@ -24,76 +24,194 @@ import android.content.Intent
 import android.content.ServiceConnection
 import android.os.IBinder
 import io.livekit.android.util.LKLog
+import kotlinx.coroutines.CancellableContinuation
+import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.suspendCancellableCoroutine
-import kotlin.coroutines.Continuation
 import kotlin.coroutines.resume
+import kotlin.coroutines.resumeWithException
 
 /**
  * Handles connecting to a [ScreenCaptureService].
  */
 internal class ScreenCaptureConnection(private val context: Context) {
+    /**
+     * True while [ScreenCaptureService] is connected and [startForeground] can reach it.
+     */
     var isBound = false
         private set
+
+    /**
+     * True from the start of a bind attempt until its matching unbind attempt. This is a wider
+     * window than [isBound]: the binding is owed an unbind even if the service never connects.
+     */
+    private var isBindRequested = false
     private var service: ScreenCaptureService? = null
-    private val queuedConnects = mutableSetOf<Continuation<Unit>>()
+    private var hasConnectedCaller = false
+    private val queuedConnects = mutableSetOf<CancellableContinuation<Unit>>()
     private val connection: ServiceConnection = object : ServiceConnection {
         override fun onServiceDisconnected(name: ComponentName) {
             LKLog.v { "Screen capture service is disconnected" }
-            isBound = false
-            service = null
+            synchronized(this@ScreenCaptureConnection) {
+                isBound = false
+                service = null
+            }
         }
 
         override fun onServiceConnected(name: ComponentName, binder: IBinder) {
             LKLog.v { "Screen capture service is connected" }
             val screenCaptureBinder = binder as ScreenCaptureService.ScreenCaptureBinder
-            service = screenCaptureBinder.service
-            handleConnect()
+            val connects = synchronized(this@ScreenCaptureConnection) {
+                if (!isBindRequested) {
+                    return
+                }
+                service = screenCaptureBinder.service
+                isBound = true
+                queuedConnects.filter { it.isActive }
+            }
+            connects.forEach { it.resume(Unit) }
         }
-    }
 
-    suspend fun connect() {
-        if (isBound) {
-            return
+        override fun onBindingDied(name: ComponentName) {
+            LKLog.w { "Screen capture service binding died" }
+            failPendingConnects("ScreenCaptureService binding died.")
         }
 
-        val intent = Intent(context, ScreenCaptureService::class.java)
-        val bound = context.bindService(intent, connection, BIND_AUTO_CREATE)
-        if (!bound) {
-            throw IllegalStateException("Failed to bind ScreenCaptureService.")
+        override fun onNullBinding(name: ComponentName) {
+            LKLog.w { "Screen capture service returned a null binding" }
+            failPendingConnects("ScreenCaptureService returned a null binding.")
         }
-        return suspendCancellableCoroutine { cont ->
-            cont.invokeOnCancellation {
-                synchronized(this) {
-                    queuedConnects.remove(cont)
-                }
-            }
+    }
+
+    /**
+     * Binds to [ScreenCaptureService] and suspends until it is connected.
+     *
+     * @throws IllegalStateException if the service could not be bound, or the binding died or
+     * came back null before the service connected.
+     * @throws SecurityException if the caller cannot access the service.
+     * @throws CancellationException if [stop] tears the connection down while connecting.
+     */
+    suspend fun connect() {
+        lateinit var continuation: CancellableContinuation<Unit>
+        suspendCancellableCoroutine { cont ->
+            continuation = cont
+            // Binding and enqueueing happen under one lock, so a concurrent stop() either
+            // precedes the bind or cancels the waiter, and never strands it against a
+            // connection that has already been unbound.
+            var outcome: Result<Unit>? = null
             synchronized(this) {
                 if (isBound) {
-                    cont.resume(Unit)
+                    outcome = Result.success(Unit)
                 } else {
                     queuedConnects.add(cont)
+                    val failure = runCatching { if (!isBindRequested) bind() }.exceptionOrNull()
+                    if (failure != null) {
+                        queuedConnects.remove(cont)
+                        outcome = Result.failure(failure)
+                    }
                 }
             }
+
+            val result = outcome
+            if (result == null) {
+                // Registered once the waiter is queued, so a continuation that was already
+                // cancelled still releases the binding it just requested.
+                cont.invokeOnCancellation { abandonConnect(cont) }
+            } else {
+                // Resumed outside the lock, since resuming runs the continuation inline.
+                cont.resumeWith(result)
+            }
         }
-    }
 
-    fun startForeground(notificationId: Int? = null, notification: Notification? = null) {
-        service?.start(notificationId, notification)
+        val connected = synchronized(this) {
+            queuedConnects.remove(continuation)
+            if (isBindRequested && isBound) {
+                hasConnectedCaller = true
+                true
+            } else {
+                if (queuedConnects.isEmpty() && !hasConnectedCaller) {
+                    unbind()
+                }
+                false
+            }
+        }
+        if (!connected) {
+            throw CancellationException("ScreenCaptureService connection was stopped.")
+        }
     }
 
-    private fun handleConnect() {
+    /**
+     * Releases a binding that no longer has anyone waiting on it. Screen share setup is routinely
+     * driven from a cancellable scope, and the caller that requested the bind does not necessarily
+     * survive to call [stop].
+     */
+    private fun abandonConnect(cont: CancellableContinuation<Unit>) {
         synchronized(this) {
-            isBound = true
-            queuedConnects.forEach { it.resume(Unit) }
-            queuedConnects.clear()
+            queuedConnects.remove(cont)
+            if (queuedConnects.isEmpty() && !hasConnectedCaller) {
+                unbind()
+            }
         }
     }
 
-    fun stop() {
-        if (isBound) {
-            context.unbindService(connection)
+    /**
+     * Releases a binding the platform has reported will never connect, and fails every caller
+     * waiting on it. Neither report is followed by [ServiceConnection.onServiceConnected], so a
+     * queued caller would otherwise stay suspended until [stop].
+     */
+    private fun failPendingConnects(message: String) {
+        val failedConnects = synchronized(this) {
+            if (!isBindRequested) {
+                return
+            }
+            unbind()
+            queuedConnects.toList().also { queuedConnects.clear() }
         }
-        service = null
+        // Resumed outside the lock, since resuming runs the continuation inline.
+        failedConnects.forEach { it.resumeWithException(IllegalStateException(message)) }
+    }
+
+    private fun bind() {
+        val intent = Intent(context, ScreenCaptureService::class.java)
+        // A connection is registered even when bindService reports failure, so the unbind is
+        // owed from the moment the call is made.
+        isBindRequested = true
+        val bound = try {
+            context.bindService(intent, connection, BIND_AUTO_CREATE)
+        } catch (e: Exception) {
+            unbind()
+            throw e
+        }
+        if (!bound) {
+            unbind()
+            throw IllegalStateException("Failed to bind ScreenCaptureService.")
+        }
+    }
+
+    private fun unbind() {
+        if (!isBindRequested) {
+            return
+        }
+        isBindRequested = false
         isBound = false
+        service = null
+        hasConnectedCaller = false
+        try {
+            context.unbindService(connection)
+        } catch (e: IllegalArgumentException) {
+            LKLog.v(e) { "Screen capture service was not bound" }
+        }
+    }
+
+    fun startForeground(notificationId: Int? = null, notification: Notification? = null) {
+        service?.start(notificationId, notification)
+    }
+
+    fun stop() {
+        val abandonedConnects = synchronized(this) {
+            unbind()
+            queuedConnects.toList().also { queuedConnects.clear() }
+        }
+        // Cancelled outside the lock, since cancellation runs handlers inline.
+        abandonedConnects.forEach { it.cancel() }
     }
 }