davidliu
Committed by GitHub

More fixes for crashes caused by using disposed track (#497)

* More fixes for crashes caused by using disposed track

* Add withRTCTrack method to refactor the isDisposed usage

* spotless
  1 +---
  2 +"client-sdk-android": patch
  3 +---
  4 +
  5 +More fixes for crashes caused by using disposed track
1 [versions] 1 [versions]
2 -webrtc = "125.6422.04" 2 +webrtc = "125.6422.05"
3 3
4 androidJainSipRi = "1.3.0-91" 4 androidJainSipRi = "1.3.0-91"
5 androidx-activity = "1.9.0" 5 androidx-activity = "1.9.0"
@@ -16,7 +16,6 @@ @@ -16,7 +16,6 @@
16 16
17 package io.livekit.android.room.track 17 package io.livekit.android.room.track
18 18
19 -import io.livekit.android.webrtc.peerconnection.executeBlockingOnRTCThread  
20 import livekit.org.webrtc.AudioTrack 19 import livekit.org.webrtc.AudioTrack
21 import livekit.org.webrtc.AudioTrackSink 20 import livekit.org.webrtc.AudioTrackSink
22 import livekit.org.webrtc.RtpReceiver 21 import livekit.org.webrtc.RtpReceiver
@@ -39,23 +38,19 @@ class RemoteAudioTrack( @@ -39,23 +38,19 @@ class RemoteAudioTrack(
39 * to use the data after this function returns. 38 * to use the data after this function returns.
40 */ 39 */
41 fun addSink(sink: AudioTrackSink) { 40 fun addSink(sink: AudioTrackSink) {
42 - executeBlockingOnRTCThread {  
43 - if (!isDisposed) { 41 + withRTCTrack {
44 rtcTrack.addSink(sink) 42 rtcTrack.addSink(sink)
45 } 43 }
46 } 44 }
47 - }  
48 45
49 /** 46 /**
50 * Removes a previously added sink. 47 * Removes a previously added sink.
51 */ 48 */
52 fun removeSink(sink: AudioTrackSink) { 49 fun removeSink(sink: AudioTrackSink) {
53 - executeBlockingOnRTCThread {  
54 - if (!isDisposed) { 50 + withRTCTrack {
55 rtcTrack.removeSink(sink) 51 rtcTrack.removeSink(sink)
56 } 52 }
57 } 53 }
58 - }  
59 54
60 /** 55 /**
61 * Sets the volume. 56 * Sets the volume.
@@ -66,10 +61,8 @@ class RemoteAudioTrack( @@ -66,10 +61,8 @@ class RemoteAudioTrack(
66 * * values greater than 1 will boost the volume of the track. 61 * * values greater than 1 will boost the volume of the track.
67 */ 62 */
68 fun setVolume(volume: Double) { 63 fun setVolume(volume: Double) {
69 - executeBlockingOnRTCThread {  
70 - if (!isDisposed) { 64 + withRTCTrack {
71 rtcTrack.setVolume(volume) 65 rtcTrack.setVolume(volume)
72 } 66 }
73 } 67 }
74 - }  
75 } 68 }
@@ -27,6 +27,9 @@ import livekit.LivekitRtc @@ -27,6 +27,9 @@ import livekit.LivekitRtc
27 import livekit.org.webrtc.MediaStreamTrack 27 import livekit.org.webrtc.MediaStreamTrack
28 import livekit.org.webrtc.RTCStatsCollectorCallback 28 import livekit.org.webrtc.RTCStatsCollectorCallback
29 import livekit.org.webrtc.RTCStatsReport 29 import livekit.org.webrtc.RTCStatsReport
  30 +import kotlin.contracts.ExperimentalContracts
  31 +import kotlin.contracts.InvocationKind
  32 +import kotlin.contracts.contract
30 33
31 abstract class Track( 34 abstract class Track(
32 name: String, 35 name: String,
@@ -50,32 +53,13 @@ abstract class Track( @@ -50,32 +53,13 @@ abstract class Track(
50 internal set 53 internal set
51 54
52 var enabled: Boolean 55 var enabled: Boolean
53 - get() = executeBlockingOnRTCThread {  
54 - if (!isDisposed) {  
55 - rtcTrack.enabled()  
56 - } else {  
57 - false  
58 - }  
59 - }  
60 - set(value) {  
61 - executeBlockingOnRTCThread {  
62 - if (!isDisposed) {  
63 - rtcTrack.setEnabled(value)  
64 - }  
65 - }  
66 - } 56 + get() = withRTCTrack(defaultValue = false) { rtcTrack.enabled() }
  57 + set(value) = withRTCTrack { rtcTrack.setEnabled(value) }
67 58
68 var statsGetter: RTCStatsGetter? = null 59 var statsGetter: RTCStatsGetter? = null
69 60
70 - /**  
71 - * [MediaStreamTrack] doesn't expose a way to tell if it's disposed. Track it here.  
72 - * This can only be safely accessed on the rtc thread.  
73 - *  
74 - * Note: [MediaStreamTrack] can still be disposed if we don't own it, so this isn't necessarily safe,  
75 - * however generally all the tracks passed into this class are owned by this class.  
76 - */  
77 - internal var isDisposed = false  
78 - private set 61 + internal val isDisposed
  62 + get() = rtcTrack.isDisposed
79 63
80 /** 64 /**
81 * Return the [RTCStatsReport] for this track, or null if none is available. 65 * Return the [RTCStatsReport] for this track, or null if none is available.
@@ -193,11 +177,31 @@ abstract class Track( @@ -193,11 +177,31 @@ abstract class Track(
193 * Disposes the track. LiveKit will generally take care of disposing tracks for you. 177 * Disposes the track. LiveKit will generally take care of disposing tracks for you.
194 */ 178 */
195 open fun dispose() { 179 open fun dispose() {
196 - executeBlockingOnRTCThread {  
197 - isDisposed = true 180 + withRTCTrack {
198 rtcTrack.dispose() 181 rtcTrack.dispose()
199 } 182 }
200 } 183 }
  184 +
  185 + @OptIn(ExperimentalContracts::class)
  186 + internal inline fun <T> withRTCTrack(crossinline action: MediaStreamTrack.() -> T) {
  187 + contract { callsInPlace(action, InvocationKind.AT_MOST_ONCE) }
  188 + withRTCTrack(Unit, action)
  189 + }
  190 +
  191 + @OptIn(ExperimentalContracts::class)
  192 + internal inline fun <T> withRTCTrack(defaultValue: T, crossinline action: MediaStreamTrack.() -> T): T {
  193 + contract { callsInPlace(action, InvocationKind.AT_MOST_ONCE) }
  194 + if (isDisposed) {
  195 + return defaultValue
  196 + }
  197 + return executeBlockingOnRTCThread {
  198 + return@executeBlockingOnRTCThread if (isDisposed) {
  199 + defaultValue
  200 + } else {
  201 + action(rtcTrack)
  202 + }
  203 + }
  204 + }
201 } 205 }
202 206
203 sealed class TrackException(message: String? = null, cause: Throwable? = null) : 207 sealed class TrackException(message: String? = null, cause: Throwable? = null) :
@@ -28,13 +28,11 @@ abstract class VideoTrack(name: String, override val rtcTrack: VideoTrack) : @@ -28,13 +28,11 @@ abstract class VideoTrack(name: String, override val rtcTrack: VideoTrack) :
28 * Add a [VideoSink] that will receive frames. 28 * Add a [VideoSink] that will receive frames.
29 */ 29 */
30 open fun addRenderer(renderer: VideoSink) { 30 open fun addRenderer(renderer: VideoSink) {
31 - executeBlockingOnRTCThread {  
32 - if (!isDisposed) { 31 + withRTCTrack {
33 sinks.add(renderer) 32 sinks.add(renderer)
34 rtcTrack.addSink(renderer) 33 rtcTrack.addSink(renderer)
35 } 34 }
36 } 35 }
37 - }  
38 36
39 /** 37 /**
40 * Remove a previously added [VideoSink]. 38 * Remove a previously added [VideoSink].
@@ -43,8 +41,8 @@ abstract class VideoTrack(name: String, override val rtcTrack: VideoTrack) : @@ -43,8 +41,8 @@ abstract class VideoTrack(name: String, override val rtcTrack: VideoTrack) :
43 executeBlockingOnRTCThread { 41 executeBlockingOnRTCThread {
44 if (!isDisposed) { 42 if (!isDisposed) {
45 rtcTrack.removeSink(renderer) 43 rtcTrack.removeSink(renderer)
46 - sinks.remove(renderer)  
47 } 44 }
  45 + sinks.remove(renderer)
48 } 46 }
49 } 47 }
50 48
@@ -47,4 +47,8 @@ class MockMediaStreamTrack( @@ -47,4 +47,8 @@ class MockMediaStreamTrack(
47 } 47 }
48 disposed = true 48 disposed = true
49 } 49 }
  50 +
  51 + override fun isDisposed(): Boolean {
  52 + return disposed
  53 + }
50 } 54 }