Skip to content

Commit 71b321a

Browse files
authored
Swallowed cancellation exception fixes (#913)
* add detekt plugin * Rethrow cancellation exceptions for coroutines * Fix potential leak for StreamSender caused by exceptions * spotless * Migrate withTimeout to withDeadline withTimeout throws a cancellation exception, cancelling the coroutine * Detekt rule for NoWithTimeout * spotless * Cleanup
1 parent afa37c7 commit 71b321a

27 files changed

Lines changed: 511 additions & 51 deletions

File tree

.changeset/breezy-mugs-raise.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"client-sdk-android": patch
3+
---
4+
5+
Fix potential leak for StreamSender caused by exceptions

.changeset/eight-numbers-fold.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"client-sdk-android": patch
3+
---
4+
5+
Rethrow cancellation exceptions for coroutines

.github/workflows/android.yml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,6 @@ jobs:
4949
- name: Grant execute permission for gradlew
5050
run: chmod +x gradlew
5151

52-
- name: Gradle clean
53-
run: ./gradlew clean
54-
5552
- name: Spotless check
5653
if: github.event_name == 'pull_request'
5754
run: |

build.gradle

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ buildscript {
2424

2525
plugins {
2626
id("io.github.gradle-nexus.publish-plugin") version "2.0.0"
27+
alias(libs.plugins.detekt) apply false
2728
}
2829

2930
subprojects {
@@ -32,6 +33,8 @@ subprojects {
3233
return
3334
}
3435

36+
def isDetektRulesModule = project.name == "livekit-detekt-rules"
37+
3538
apply plugin: "com.diffplug.spotless"
3639
spotless {
3740
// optional: limit format enforcement to just the files changed by this feature branch
@@ -75,6 +78,28 @@ subprojects {
7578
}
7679
}
7780

81+
if (!isDetektRulesModule) {
82+
apply plugin: "io.gitlab.arturbosch.detekt"
83+
detekt {
84+
parallel = true
85+
buildUponDefaultConfig = true
86+
allRules = false
87+
config.from(files("$rootDir/config/detekt/detekt.yml"))
88+
reports {
89+
html.required.set(true)
90+
md.required.set(true)
91+
}
92+
}
93+
94+
dependencies {
95+
detektPlugins project(":livekit-detekt-rules")
96+
}
97+
98+
tasks.withType(io.gitlab.arturbosch.detekt.Detekt).configureEach {
99+
pluginClasspath.setFrom(configurations.detektPlugins)
100+
}
101+
}
102+
78103
// From Gradle 8 onwards, Kapt no longer automatically picks up jvmTarget
79104
// from normal KotlinOptions. Must be manually set.
80105
// JvmToolchain should not be used since it changes the actual JDK used.

config/detekt/detekt.yml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# Detekt: smell-oriented checks only. Formatting and naming conventions are handled by Spotless/ktlint.
2+
config:
3+
validation: true
4+
warningsAsErrors: true
5+
excludes: ''
6+
7+
style:
8+
active: false
9+
10+
naming:
11+
active: false
12+
13+
coroutines:
14+
SuspendFunSwallowedCancellation:
15+
active: true
16+
17+
livekit-rules:
18+
NoWithTimeout:
19+
active: true
20+
21+
exceptions:
22+
TooGenericExceptionCaught:
23+
active: false

gradle.properties

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,15 @@ android.enableJetifier=false
2121
kotlin.code.style=official
2222

2323
org.gradle.caching=true
24+
org.gradle.configuration-cache=true
25+
org.gradle.configureondemand=true
26+
org.gradle.daemon=true
27+
28+
kotlin.incremental=true
29+
kapt.incremental.apt=true
30+
kapt.use.worker.api=true
31+
32+
2433
###############################################################
2534

2635
GROUP=io.livekit
@@ -62,8 +71,3 @@ RELEASE_SIGNING_ENABLED=true
6271
livekitUrl=
6372
livekitApiKey=
6473
livekitApiSecret=
65-
66-
org.gradle.configuration-cache=true
67-
kotlin.incremental=true
68-
org.gradle.configureondemand=true
69-
org.gradle.daemon=true

gradle/libs.versions.toml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ androidx-lifecycle = "2.8.0"
1010
audioswitch = "70efa204dda4f468f989b4b3e1b5ecb6ffe56ceb"
1111
autoService = '1.0.1'
1212
coroutines = "1.6.0"
13+
detekt = "1.23.8"
1314
dagger = "2.46"
1415
groupie = "2.9.0"
1516
junit-lib = "4.13.2"
@@ -29,6 +30,8 @@ material = "1.12.0"
2930
viewpager2 = "1.0.0"
3031
noise = "2.0.0"
3132
lifecycleProcess = "2.8.7"
33+
agp = "8.7.2"
34+
kotlin = "1.9.25"
3235

3336
[libraries]
3437
android-jain-sip-ri = { module = "javax.sip:android-jain-sip-ri", version.ref = "androidJainSipRi" }
@@ -109,5 +112,11 @@ androidx-ui-test-junit4 = { group = "androidx.compose.ui", name = "ui-test-junit
109112
androidx-material3 = { group = "androidx.compose.material3", name = "material3" }
110113
lifecycle-process = { group = "androidx.lifecycle", name = "lifecycle-process", version.ref = "lifecycleProcess" }
111114

115+
detekt-api = { module = "io.gitlab.arturbosch.detekt:detekt-api", version.ref = "detekt" }
116+
detekt-test = { module = "io.gitlab.arturbosch.detekt:detekt-test", version.ref = "detekt" }
117+
112118
[plugins]
119+
detekt = { id = "io.gitlab.arturbosch.detekt", version.ref = "detekt" }
120+
android-library = { id = "com.android.library", version.ref = "agp" }
121+
kotlin-android = { id = "org.jetbrains.kotlin.android", version.ref = "kotlin" }
113122

livekit-android-sdk/src/main/java/io/livekit/android/audio/PreconnectAudioBuffer.kt

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2025 LiveKit, Inc.
2+
* Copyright 2025-2026 LiveKit, Inc.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -24,9 +24,11 @@ import io.livekit.android.events.collect
2424
import io.livekit.android.room.ConnectionState
2525
import io.livekit.android.room.Room
2626
import io.livekit.android.room.datastream.StreamBytesOptions
27+
import io.livekit.android.room.datastream.outgoing.useStreamSender
2728
import io.livekit.android.room.participant.Participant
2829
import io.livekit.android.util.LKLog
2930
import io.livekit.android.util.flow
31+
import io.livekit.android.util.rethrowIfCancellationSignal
3032
import kotlinx.coroutines.CoroutineScope
3133
import kotlinx.coroutines.Job
3234
import kotlinx.coroutines.cancel
@@ -150,14 +152,15 @@ internal constructor(timeout: Duration) : AudioTrackSink {
150152
),
151153
)
152154

153-
try {
154-
val result = sender.write(audioData)
155+
val sendResult = useStreamSender(sender) {
156+
val result = write(audioData)
155157
if (result.isFailure) {
156158
result.exceptionOrNull()?.let { throw it }
157159
}
158-
sender.close()
159-
} catch (e: Exception) {
160-
sender.close(e.localizedMessage)
160+
close()
161+
}
162+
if (sendResult.isFailure) {
163+
return
161164
}
162165

163166
val samples = audioData.size / (numberOfChannels * bitsPerSample / 8)
@@ -257,6 +260,7 @@ suspend fun <T> Room.withPreconnectAudio(
257260
)
258261
sentIdentities.add(identity)
259262
} catch (e: Exception) {
263+
e.rethrowIfCancellationSignal()
260264
LKLog.w(e) { "Error occurred while sending the audio preconnect data." }
261265
onError?.invoke(e)
262266
}
@@ -295,6 +299,7 @@ suspend fun <T> Room.withPreconnectAudio(
295299
try {
296300
retValue = operation.invoke()
297301
} catch (e: Exception) {
302+
e.rethrowIfCancellationSignal()
298303
cancel()
299304
throw e
300305
}
@@ -361,6 +366,7 @@ internal suspend fun Room.startPreconnectAudioJob(
361366
)
362367
sentIdentities.add(identity)
363368
} catch (e: Exception) {
369+
e.rethrowIfCancellationSignal()
364370
LKLog.w(e) { "Error occurred while sending the audio preconnect data." }
365371
}
366372
}

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ import io.livekit.android.util.TTLMap
4747
import io.livekit.android.util.flow
4848
import io.livekit.android.util.flowDelegate
4949
import io.livekit.android.util.nullSafe
50+
import io.livekit.android.util.rethrowIfCancellationSignal
5051
import io.livekit.android.util.withCheckLock
52+
import io.livekit.android.util.withDeadline
5153
import io.livekit.android.webrtc.DataChannelManager
5254
import io.livekit.android.webrtc.DataPacketBuffer
5355
import io.livekit.android.webrtc.DataPacketItem
@@ -71,7 +73,6 @@ import kotlinx.coroutines.runBlocking
7173
import kotlinx.coroutines.suspendCancellableCoroutine
7274
import kotlinx.coroutines.sync.Mutex
7375
import kotlinx.coroutines.sync.withLock
74-
import kotlinx.coroutines.withTimeout
7576
import kotlinx.coroutines.withTimeoutOrNull
7677
import kotlinx.coroutines.yield
7778
import livekit.LivekitModels
@@ -397,7 +398,7 @@ internal constructor(
397398
}
398399

399400
// Suspend until signal client receives message confirming track publication.
400-
return withTimeout(20.seconds) {
401+
return withDeadline(20.seconds) {
401402
suspendCancellableCoroutine { cont ->
402403
synchronized(pendingTrackResolvers) {
403404
pendingTrackResolvers[cid] = cont
@@ -540,6 +541,7 @@ internal constructor(
540541
try {
541542
url = regionUrlProvider?.getNextBestRegionUrl() ?: url
542543
} catch (e: Exception) {
544+
e.rethrowIfCancellationSignal()
543545
LKLog.d(e) { "Exception while getting next best region url while reconnecting." }
544546
}
545547
}
@@ -589,6 +591,7 @@ internal constructor(
589591
listener?.onFullReconnecting()
590592
joinImpl(url!!, token, connectOptions, lastRoomOptions ?: RoomOptions())
591593
} catch (e: Exception) {
594+
e.rethrowIfCancellationSignal()
592595
LKLog.w(e) { "Error during reconnection." }
593596
// reconnect failed, retry.
594597
continue
@@ -612,6 +615,7 @@ internal constructor(
612615
}
613616
client.onReadyForResponses()
614617
} catch (e: Exception) {
618+
e.rethrowIfCancellationSignal()
615619
LKLog.w(e) { "Error during reconnection." }
616620
// ws reconnect failed, retry.
617621
continue
@@ -716,6 +720,7 @@ internal constructor(
716720
try {
717721
ensurePublisherConnected(dataPacket.kind)
718722
} catch (e: Exception) {
723+
e.rethrowIfCancellationSignal()
719724
return Result.failure(e)
720725
}
721726

@@ -769,6 +774,7 @@ internal constructor(
769774

770775
channel.send(buf)
771776
} catch (e: Exception) {
777+
e.rethrowIfCancellationSignal()
772778
return Result.failure(e)
773779
}
774780
return Result.success(Unit)
@@ -802,6 +808,7 @@ internal constructor(
802808
try {
803809
ensurePublisherConnected(kind)
804810
} catch (e: Exception) {
811+
e.rethrowIfCancellationSignal()
805812
return
806813
}
807814
val manager = dataChannelManagerForKind(kind) ?: return

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ import io.livekit.android.util.LKLog
7575
import io.livekit.android.util.flow
7676
import io.livekit.android.util.flowDelegate
7777
import io.livekit.android.util.invoke
78+
import io.livekit.android.util.rethrowIfCancellationSignal
7879
import io.livekit.android.webrtc.getFilteredStats
7980
import kotlinx.coroutines.CancellationException
8081
import kotlinx.coroutines.CoroutineDispatcher
@@ -414,6 +415,7 @@ constructor(
414415
connectionWarmer.fetch(url)
415416
}
416417
} catch (e: Exception) {
418+
e.rethrowIfCancellationSignal()
417419
LKLog.e(e) { "Error while preparing connection:" }
418420
}
419421
}
@@ -498,6 +500,7 @@ constructor(
498500
try {
499501
regionUrlProvider?.fetchRegionSettings()
500502
} catch (e: Exception) {
503+
e.rethrowIfCancellationSignal()
501504
LKLog.w(e) { "could not fetch region settings" }
502505
}
503506
}
@@ -513,9 +516,7 @@ constructor(
513516
engine.regionUrlProvider = regionUrlProvider
514517
engine.join(connectUrl, token, options, roomOptions)
515518
} catch (e: Exception) {
516-
if (e is CancellationException) {
517-
throw e // rethrow to properly cancel.
518-
}
519+
e.rethrowIfCancellationSignal()
519520

520521
nextUrl = regionUrlProvider?.getNextBestRegionUrl()
521522
if (nextUrl != null) {

0 commit comments

Comments
 (0)