From 40e188165fbf367880875d5f55fdee2da6bc04eb Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Sun, 26 Jul 2026 11:31:18 +0300 Subject: [PATCH 1/2] Fixed ScreenCaptureService staying bound when screen share setup is cancelled ScreenCaptureConnection recorded a binding only once onServiceConnected arrived. A coroutine cancelled before connect() returned left the ServiceConnection registered, so 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 when 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. Both cover the documented case where bindService leaves a connection registered while reporting failure or throwing. Two paths could also leave connect() suspended forever. A stop() racing a connect could unbind and snapshot an empty waiter set just before the caller enqueued itself, so requesting the bind and registering the waiter now happen under one lock. A failed bindService left the state claiming a bind was in flight, which the next caller would wait on with nothing pending. --- .../screen-capture-connection-bind-leak.md | 7 + .../screencapture/ScreenCaptureConnection.kt | 157 ++++++++++++++---- .../ScreenCaptureConnectionTest.kt | 150 +++++++++++++++++ 3 files changed, 280 insertions(+), 34 deletions(-) create mode 100644 .changeset/screen-capture-connection-bind-leak.md create mode 100644 livekit-android-test/src/test/java/io/livekit/android/room/track/screencapture/ScreenCaptureConnectionTest.kt diff --git a/.changeset/screen-capture-connection-bind-leak.md b/.changeset/screen-capture-connection-bind-leak.md new file mode 100644 index 000000000..b3de0eb7e --- /dev/null +++ b/.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. diff --git a/livekit-android-sdk/src/main/java/io/livekit/android/room/track/screencapture/ScreenCaptureConnection.kt b/livekit-android-sdk/src/main/java/io/livekit/android/room/track/screencapture/ScreenCaptureConnection.kt index 19c5225e2..977304545 100644 --- a/livekit-android-sdk/src/main/java/io/livekit/android/room/track/screencapture/ScreenCaptureConnection.kt +++ b/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,165 @@ 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 /** * 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>() + private var hasConnectedCaller = false + private val queuedConnects = mutableSetOf>() 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) } } } + /** + * Binds to [ScreenCaptureService] and suspends until it is connected. + * + * @throws IllegalStateException if the service could not be bound. + * @throws SecurityException if the caller cannot access the service. + * @throws CancellationException if [stop] tears the connection down while connecting. + */ suspend fun connect() { - if (isBound) { - return - } - - val intent = Intent(context, ScreenCaptureService::class.java) - val bound = context.bindService(intent, connection, BIND_AUTO_CREATE) - if (!bound) { - throw IllegalStateException("Failed to bind ScreenCaptureService.") - } - return suspendCancellableCoroutine { cont -> - cont.invokeOnCancellation { - synchronized(this) { - queuedConnects.remove(cont) - } - } + lateinit var continuation: CancellableContinuation + 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? = 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) { 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) + 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 } - service = null + 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() } } } diff --git a/livekit-android-test/src/test/java/io/livekit/android/room/track/screencapture/ScreenCaptureConnectionTest.kt b/livekit-android-test/src/test/java/io/livekit/android/room/track/screencapture/ScreenCaptureConnectionTest.kt new file mode 100644 index 000000000..99909ca89 --- /dev/null +++ b/livekit-android-test/src/test/java/io/livekit/android/room/track/screencapture/ScreenCaptureConnectionTest.kt @@ -0,0 +1,150 @@ +/* + * Copyright 2025-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.track.screencapture + +import android.app.Application +import android.content.ComponentName +import androidx.test.core.app.ApplicationProvider +import io.livekit.android.test.BaseTest +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.cancelAndJoin +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.runCurrent +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf + +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +class ScreenCaptureConnectionTest : BaseTest() { + + private val application = ApplicationProvider.getApplicationContext() + + private fun connectService() { + shadowOf(application).boundServiceConnections.single().onServiceConnected( + ComponentName(application, ScreenCaptureService::class.java), + ScreenCaptureService().onBind(null), + ) + } + + private fun disconnectService() { + shadowOf(application).boundServiceConnections.single().onServiceDisconnected( + ComponentName(application, ScreenCaptureService::class.java), + ) + } + + @Test + fun cancellingConnectUnbindsBeforeServiceConnects() = runTest { + val screenCaptureConnection = ScreenCaptureConnection(application) + + val connectJob = launch { screenCaptureConnection.connect() } + assertEquals(1, shadowOf(application).boundServiceConnections.size) + + // The caller that requested the bind is not guaranteed to survive to call stop(). + connectJob.cancelAndJoin() + + assertEquals(1, shadowOf(application).unboundServiceConnections.size) + assertEquals(0, shadowOf(application).boundServiceConnections.size) + } + + @Test + fun connectRebindsAfterAnEarlierConnectWasCancelled() = runTest { + val screenCaptureConnection = ScreenCaptureConnection(application) + + launch { screenCaptureConnection.connect() }.cancelAndJoin() + assertEquals(0, shadowOf(application).boundServiceConnections.size) + + val connectJob = launch { screenCaptureConnection.connect() } + + // A cancelled connect must not leave the state claiming a bind is still in flight, + // which would leave this caller waiting on a connection that was already released. + assertEquals(1, shadowOf(application).boundServiceConnections.size) + + connectJob.cancelAndJoin() + } + + @Test + fun cancellingConnectAfterServiceConnectsUnbindsBeforeResumption() = runTest { + val screenCaptureConnection = ScreenCaptureConnection(application) + val connectJob = launch(StandardTestDispatcher(testScheduler)) { + screenCaptureConnection.connect() + } + runCurrent() + + connectService() + connectJob.cancel() + + runCurrent() + connectJob.join() + assertEquals(0, shadowOf(application).boundServiceConnections.size) + } + + @Test + fun connectKeepsServiceBoundOnceConnected() = runTest { + val screenCaptureConnection = ScreenCaptureConnection(application) + val connectJob = launch(StandardTestDispatcher(testScheduler)) { + screenCaptureConnection.connect() + } + runCurrent() + + connectService() + runCurrent() + connectJob.join() + + assertTrue(screenCaptureConnection.isBound) + assertEquals(1, shadowOf(application).boundServiceConnections.size) + } + + @Test + fun cancellingConnectKeepsBindingHeldByAnEarlierCaller() = runTest { + val screenCaptureConnection = ScreenCaptureConnection(application) + val firstConnect = launch(StandardTestDispatcher(testScheduler)) { + screenCaptureConnection.connect() + } + runCurrent() + connectService() + runCurrent() + firstConnect.join() + + // A dropped service connection leaves the binding in place, so a second caller waits on + // it rather than binding again. + disconnectService() + val secondConnect = launch(StandardTestDispatcher(testScheduler)) { + screenCaptureConnection.connect() + } + runCurrent() + secondConnect.cancelAndJoin() + + // Releasing on cancellation must not tear down a binding the first caller still holds. + assertEquals(1, shadowOf(application).boundServiceConnections.size) + } + + @Test + fun stopEndsPendingConnect() = runTest { + val screenCaptureConnection = ScreenCaptureConnection(application) + + val connectJob = launch { screenCaptureConnection.connect() } + screenCaptureConnection.stop() + + connectJob.join() + assertEquals(0, shadowOf(application).boundServiceConnections.size) + } +} From f275e98c4911be69f5a40bbcc95d420581598a3d Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Mon, 27 Jul 2026 11:21:25 +0300 Subject: [PATCH 2/2] Failed pending connects when the binding dies or comes back null Neither onBindingDied nor onNullBinding is followed by onServiceConnected, so a queued caller would stay suspended until stop(). Both now release the binding and fail the waiters with IllegalStateException. --- .../screencapture/ScreenCaptureConnection.kt | 31 +++++++++++++- .../ScreenCaptureConnectionTest.kt | 41 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/livekit-android-sdk/src/main/java/io/livekit/android/room/track/screencapture/ScreenCaptureConnection.kt b/livekit-android-sdk/src/main/java/io/livekit/android/room/track/screencapture/ScreenCaptureConnection.kt index 977304545..f4bbfa1a3 100644 --- a/livekit-android-sdk/src/main/java/io/livekit/android/room/track/screencapture/ScreenCaptureConnection.kt +++ b/livekit-android-sdk/src/main/java/io/livekit/android/room/track/screencapture/ScreenCaptureConnection.kt @@ -28,6 +28,7 @@ import kotlinx.coroutines.CancellableContinuation import kotlinx.coroutines.CancellationException import kotlinx.coroutines.suspendCancellableCoroutine import kotlin.coroutines.resume +import kotlin.coroutines.resumeWithException /** * Handles connecting to a [ScreenCaptureService]. @@ -69,12 +70,23 @@ internal class ScreenCaptureConnection(private val context: Context) { } connects.forEach { it.resume(Unit) } } + + override fun onBindingDied(name: ComponentName) { + LKLog.w { "Screen capture service binding died" } + failPendingConnects("ScreenCaptureService binding died.") + } + + override fun onNullBinding(name: ComponentName) { + LKLog.w { "Screen capture service returned a null binding" } + failPendingConnects("ScreenCaptureService returned a null binding.") + } } /** * Binds to [ScreenCaptureService] and suspends until it is connected. * - * @throws IllegalStateException if the service could not be bound. + * @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. */ @@ -141,6 +153,23 @@ internal class ScreenCaptureConnection(private val context: Context) { } } + /** + * 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() } + } + // 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 diff --git a/livekit-android-test/src/test/java/io/livekit/android/room/track/screencapture/ScreenCaptureConnectionTest.kt b/livekit-android-test/src/test/java/io/livekit/android/room/track/screencapture/ScreenCaptureConnectionTest.kt index 99909ca89..36960a391 100644 --- a/livekit-android-test/src/test/java/io/livekit/android/room/track/screencapture/ScreenCaptureConnectionTest.kt +++ b/livekit-android-test/src/test/java/io/livekit/android/room/track/screencapture/ScreenCaptureConnectionTest.kt @@ -137,6 +137,47 @@ class ScreenCaptureConnectionTest : BaseTest() { assertEquals(1, shadowOf(application).boundServiceConnections.size) } + @Test + fun nullBindingFailsPendingConnect() = runTest { + val screenCaptureConnection = ScreenCaptureConnection(application) + + var failure: Throwable? = null + val connectJob = launch { + failure = runCatching { screenCaptureConnection.connect() }.exceptionOrNull() + } + shadowOf(application).boundServiceConnections.single().onNullBinding( + ComponentName(application, ScreenCaptureService::class.java), + ) + + connectJob.join() + // A null binding is never followed by onServiceConnected, so the caller must fail + // rather than stay suspended. + assertTrue(failure is IllegalStateException) + assertEquals(0, shadowOf(application).boundServiceConnections.size) + } + + @Test + fun deadBindingFailsPendingConnectAndReleasesTheBinding() = runTest { + val screenCaptureConnection = ScreenCaptureConnection(application) + + var failure: Throwable? = null + val connectJob = launch { + failure = runCatching { screenCaptureConnection.connect() }.exceptionOrNull() + } + shadowOf(application).boundServiceConnections.single().onBindingDied( + ComponentName(application, ScreenCaptureService::class.java), + ) + + connectJob.join() + assertTrue(failure is IllegalStateException) + + // A dead binding never reconnects, so the next caller must bind fresh instead of + // waiting on it. + val secondConnect = launch { screenCaptureConnection.connect() } + assertEquals(1, shadowOf(application).boundServiceConnections.size) + secondConnect.cancelAndJoin() + } + @Test fun stopEndsPendingConnect() = runTest { val screenCaptureConnection = ScreenCaptureConnection(application)