More guarding of rtc api usages (#488)

* More guarding of rtc api usages

* spotless

* changeset

+---
+"client-sdk-android": patch
+---
+
+More guarding of rtc api usages to prevent crashes

@@ -34,7 +34,17 @@ import io.livekit.android.util.LoggingLevel
 import io.livekit.android.webrtc.CustomAudioProcessingFactory
 import io.livekit.android.webrtc.CustomAudioProcessingFactory
 import io.livekit.android.webrtc.CustomVideoDecoderFactory
 import io.livekit.android.webrtc.CustomVideoDecoderFactory
 import io.livekit.android.webrtc.CustomVideoEncoderFactory
 import io.livekit.android.webrtc.CustomVideoEncoderFactory
-import livekit.org.webrtc.*
+import io.livekit.android.webrtc.peerconnection.executeBlockingOnRTCThread
+import livekit.org.webrtc.AudioProcessingFactory
+import livekit.org.webrtc.EglBase
+import livekit.org.webrtc.Logging
+import livekit.org.webrtc.MediaStreamTrack
+import livekit.org.webrtc.PeerConnectionFactory
+import livekit.org.webrtc.RtpCapabilities
+import livekit.org.webrtc.SoftwareVideoDecoderFactory
+import livekit.org.webrtc.SoftwareVideoEncoderFactory
+import livekit.org.webrtc.VideoDecoderFactory
+import livekit.org.webrtc.VideoEncoderFactory
 import livekit.org.webrtc.audio.AudioDeviceModule
 import livekit.org.webrtc.audio.AudioDeviceModule
 import livekit.org.webrtc.audio.JavaAudioDeviceModule
 import livekit.org.webrtc.audio.JavaAudioDeviceModule
 import timber.log.Timber
 import timber.log.Timber
@@ -295,18 +305,20 @@ internal object RTCModule {
         memoryManager: CloseableManager,
         memoryManager: CloseableManager,
         audioProcessingFactory: AudioProcessingFactory,
         audioProcessingFactory: AudioProcessingFactory,
     ): PeerConnectionFactory {
     ): PeerConnectionFactory {
-        return PeerConnectionFactory.builder()
-            .setAudioDeviceModule(audioDeviceModule)
-            .setAudioProcessingFactory(audioProcessingFactory)
-            .setVideoEncoderFactory(videoEncoderFactory)
-            .setVideoDecoderFactory(videoDecoderFactory)
-            .apply {
-                if (peerConnectionFactoryOptions != null) {
-                    setOptions(peerConnectionFactoryOptions)
+        return executeBlockingOnRTCThread {
+            PeerConnectionFactory.builder()
+                .setAudioDeviceModule(audioDeviceModule)
+                .setAudioProcessingFactory(audioProcessingFactory)
+                .setVideoEncoderFactory(videoEncoderFactory)
+                .setVideoDecoderFactory(videoDecoderFactory)
+                .apply {
+                    if (peerConnectionFactoryOptions != null) {
+                        setOptions(peerConnectionFactoryOptions)
+                    }
                 }
                 }
-            }
-            .createPeerConnectionFactory()
-            .apply { memoryManager.registerClosable { dispose() } }
+                .createPeerConnectionFactory()
+                .apply { memoryManager.registerClosable { dispose() } }
+        }
     }
     }
 
 
     @Provides
     @Provides

@@ -23,7 +23,11 @@ import dagger.assisted.Assisted
 import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
 import dagger.assisted.AssistedInject
 import io.livekit.android.dagger.InjectionNames
 import io.livekit.android.dagger.InjectionNames
-import io.livekit.android.room.util.*
+import io.livekit.android.room.util.MediaConstraintKeys
+import io.livekit.android.room.util.createOffer
+import io.livekit.android.room.util.findConstraint
+import io.livekit.android.room.util.setLocalDescription
+import io.livekit.android.room.util.setRemoteDescription
 import io.livekit.android.util.Either
 import io.livekit.android.util.Either
 import io.livekit.android.util.LKLog
 import io.livekit.android.util.LKLog
 import io.livekit.android.util.debounce
 import io.livekit.android.util.debounce
@@ -40,9 +44,14 @@ import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.runBlocking
-import livekit.org.webrtc.*
+import livekit.org.webrtc.IceCandidate
+import livekit.org.webrtc.MediaConstraints
+import livekit.org.webrtc.PeerConnection
 import livekit.org.webrtc.PeerConnection.RTCConfiguration
 import livekit.org.webrtc.PeerConnection.RTCConfiguration
 import livekit.org.webrtc.PeerConnection.SignalingState
 import livekit.org.webrtc.PeerConnection.SignalingState
+import livekit.org.webrtc.PeerConnectionFactory
+import livekit.org.webrtc.RtpTransceiver
+import livekit.org.webrtc.SessionDescription
 import java.util.concurrent.atomic.AtomicBoolean
 import java.util.concurrent.atomic.AtomicBoolean
 import javax.inject.Named
 import javax.inject.Named
 import kotlin.contracts.ExperimentalContracts
 import kotlin.contracts.ExperimentalContracts
@@ -67,10 +76,12 @@ constructor(
     private val coroutineScope = CoroutineScope(ioDispatcher + SupervisorJob())
     private val coroutineScope = CoroutineScope(ioDispatcher + SupervisorJob())
 
 
     @VisibleForTesting
     @VisibleForTesting
-    internal val peerConnection: PeerConnection = connectionFactory.createPeerConnection(
-        config,
-        pcObserver,
-    ) ?: throw IllegalStateException("peer connection creation failed?")
+    internal val peerConnection: PeerConnection = executeBlockingOnRTCThread {
+        connectionFactory.createPeerConnection(
+            config,
+            pcObserver,
+        ) ?: throw IllegalStateException("peer connection creation failed?")
+    }
     private val pendingCandidates = mutableListOf<IceCandidate>()
     private val pendingCandidates = mutableListOf<IceCandidate>()
     private var restartingIce: Boolean = false
     private var restartingIce: Boolean = false
 
 

@@ -18,6 +18,8 @@ package io.livekit.android.room.participant
 
 
 import io.livekit.android.events.ParticipantEvent
 import io.livekit.android.events.ParticipantEvent
 import io.livekit.android.room.SignalClient
 import io.livekit.android.room.SignalClient
+import io.livekit.android.room.track.KIND_AUDIO
+import io.livekit.android.room.track.KIND_VIDEO
 import io.livekit.android.room.track.RemoteAudioTrack
 import io.livekit.android.room.track.RemoteAudioTrack
 import io.livekit.android.room.track.RemoteTrackPublication
 import io.livekit.android.room.track.RemoteTrackPublication
 import io.livekit.android.room.track.RemoteVideoTrack
 import io.livekit.android.room.track.RemoteVideoTrack
@@ -215,9 +217,4 @@ class RemoteParticipant(
     internal fun onDataReceived(data: ByteArray, topic: String?) {
     internal fun onDataReceived(data: ByteArray, topic: String?) {
         eventBus.postEvent(ParticipantEvent.DataReceived(this, data, topic), scope)
         eventBus.postEvent(ParticipantEvent.DataReceived(this, data, topic), scope)
     }
     }
-
-    companion object {
-        private const val KIND_AUDIO = "audio"
-        private const val KIND_VIDEO = "video"
-    }
 }
 }

@@ -29,7 +29,6 @@ import io.livekit.android.room.participant.LocalParticipant
 import io.livekit.android.util.FlowObservable
 import io.livekit.android.util.FlowObservable
 import io.livekit.android.util.flow
 import io.livekit.android.util.flow
 import io.livekit.android.util.flowDelegate
 import io.livekit.android.util.flowDelegate
-import io.livekit.android.webrtc.peerconnection.executeBlockingOnRTCThread
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.SupervisorJob
@@ -64,11 +63,6 @@ constructor(
      * To only be used for flow delegate scoping, and should not be cancelled.
      * To only be used for flow delegate scoping, and should not be cancelled.
      **/
      **/
     private val delegateScope = CoroutineScope(dispatcher + SupervisorJob())
     private val delegateScope = CoroutineScope(dispatcher + SupervisorJob())
-    var enabled: Boolean
-        get() = executeBlockingOnRTCThread { rtcTrack.enabled() }
-        set(value) {
-            executeBlockingOnRTCThread { rtcTrack.setEnabled(value) }
-        }
 
 
     internal var transceiver: RtpTransceiver? = null
     internal var transceiver: RtpTransceiver? = null
     internal val sender: RtpSender?
     internal val sender: RtpSender?

@@ -40,7 +40,7 @@ class LocalTrackPublication(
 
 
             val mediaTrack = track ?: return
             val mediaTrack = track ?: return
 
 
-            mediaTrack.rtcTrack.setEnabled(!muted)
+            mediaTrack.enabled = !muted
             super.muted = muted
             super.muted = muted
 
 
             // send updates to server
             // send updates to server

@@ -254,6 +254,11 @@ constructor(
      * Restart a track with new options.
      * Restart a track with new options.
      */
      */
     fun restartTrack(options: LocalVideoTrackOptions = defaultsManager.videoTrackCaptureDefaults.copy()) {
     fun restartTrack(options: LocalVideoTrackOptions = defaultsManager.videoTrackCaptureDefaults.copy()) {
+        if (isDisposed) {
+            LKLog.e { "Attempting to restart track that was already disposed, aborting." }
+            return
+        }
+
         val oldCapturer = capturer
         val oldCapturer = capturer
         val oldSource = source
         val oldSource = source
         val oldRtcTrack = rtcTrack
         val oldRtcTrack = rtcTrack

@@ -40,7 +40,9 @@ class RemoteAudioTrack(
      */
      */
     fun addSink(sink: AudioTrackSink) {
     fun addSink(sink: AudioTrackSink) {
         executeBlockingOnRTCThread {
         executeBlockingOnRTCThread {
-            rtcTrack.addSink(sink)
+            if (!isDisposed) {
+                rtcTrack.addSink(sink)
+            }
         }
         }
     }
     }
 
 
@@ -49,7 +51,9 @@ class RemoteAudioTrack(
      */
      */
     fun removeSink(sink: AudioTrackSink) {
     fun removeSink(sink: AudioTrackSink) {
         executeBlockingOnRTCThread {
         executeBlockingOnRTCThread {
-            rtcTrack.removeSink(sink)
+            if (!isDisposed) {
+                rtcTrack.removeSink(sink)
+            }
         }
         }
     }
     }
 
 
@@ -63,7 +67,9 @@ class RemoteAudioTrack(
      */
      */
     fun setVolume(volume: Double) {
     fun setVolume(volume: Double) {
         executeBlockingOnRTCThread {
         executeBlockingOnRTCThread {
-            rtcTrack.setVolume(volume)
+            if (!isDisposed) {
+                rtcTrack.setVolume(volume)
+            }
         }
         }
     }
     }
 }
 }

@@ -49,6 +49,22 @@ abstract class Track(
     }
     }
         internal set
         internal set
 
 
+    var enabled: Boolean
+        get() = executeBlockingOnRTCThread {
+            if (!isDisposed) {
+                rtcTrack.enabled()
+            } else {
+                false
+            }
+        }
+        set(value) {
+            executeBlockingOnRTCThread {
+                if (!isDisposed) {
+                    rtcTrack.setEnabled(value)
+                }
+            }
+        }
+
     var statsGetter: RTCStatsGetter? = null
     var statsGetter: RTCStatsGetter? = null
 
 
     /**
     /**
@@ -163,18 +179,14 @@ abstract class Track(
      * Starts the track.
      * Starts the track.
      */
      */
     open fun start() {
     open fun start() {
-        executeBlockingOnRTCThread {
-            rtcTrack.setEnabled(true)
-        }
+        enabled = true
     }
     }
 
 
     /**
     /**
      * Stops the track.
      * Stops the track.
      */
      */
     open fun stop() {
     open fun stop() {
-        executeBlockingOnRTCThread {
-            rtcTrack.setEnabled(false)
-        }
+        enabled = false
     }
     }
 
 
     /**
     /**
@@ -205,3 +217,6 @@ sealed class TrackException(message: String? = null, cause: Throwable? = null) :
     class PublishException(message: String? = null, cause: Throwable? = null) :
     class PublishException(message: String? = null, cause: Throwable? = null) :
         TrackException(message, cause)
         TrackException(message, cause)
 }
 }
+
+public const val KIND_AUDIO = "audio"
+public const val KIND_VIDEO = "video"

@@ -24,12 +24,6 @@ abstract class VideoTrack(name: String, override val rtcTrack: VideoTrack) :
     Track(name, Kind.VIDEO, rtcTrack) {
     Track(name, Kind.VIDEO, rtcTrack) {
     protected val sinks: MutableList<VideoSink> = ArrayList()
     protected val sinks: MutableList<VideoSink> = ArrayList()
 
 
-    var enabled: Boolean
-        get() = rtcTrack.enabled()
-        set(value) {
-            rtcTrack.setEnabled(value)
-        }
-
     /**
     /**
      * Add a [VideoSink] that will receive frames.
      * Add a [VideoSink] that will receive frames.
      */
      */