Merge branch 'charlesmchen/callThreadSafety3'

Signal/src/call/CallService.swift

@@ -271,7 +271,7 @@ protocol CallServiceObserver: class {
             return Promise(error: CallError.assertionError(description: errorDescription))
         }
 
-        return getIceServers().then() { iceServers -> Promise<HardenedRTCSessionDescription> in
+        return getIceServers().then { iceServers -> Promise<HardenedRTCSessionDescription> in
             Logger.debug("\(self.TAG) got ice servers:\(iceServers)")
 
             let peerConnectionClient = PeerConnectionClient(iceServers: iceServers, delegate: self)
@@ -281,16 +281,17 @@ protocol CallServiceObserver: class {
             peerConnectionClient.createSignalingDataChannel()
 
             assert(self.peerConnectionClient == nil, "Unexpected PeerConnectionClient instance")
+            Logger.debug("\(self.TAG) setting peerConnectionClient in \(#function)")
             self.peerConnectionClient = peerConnectionClient
 
             return self.peerConnectionClient!.createOffer()
-        }.then() { (sessionDescription: HardenedRTCSessionDescription) -> Promise<Void> in
+        }.then { (sessionDescription: HardenedRTCSessionDescription) -> Promise<Void> in
-            return self.peerConnectionClient!.setLocalSessionDescription(sessionDescription).then() {
+            return self.peerConnectionClient!.setLocalSessionDescription(sessionDescription).then {
                 let offerMessage = OWSCallOfferMessage(callId: call.signalingId, sessionDescription: sessionDescription.sdp)
                 let callMessage = OWSOutgoingCallMessage(thread: thread, offerMessage: offerMessage)
                 return self.messageSender.sendCallMessage(callMessage)
             }
-        }.catch() { error in
+        }.catch { error in
             Logger.error("\(self.TAG) placing call failed with error: \(error)")
 
             if let callError = error as? CallError {
@@ -338,7 +339,7 @@ protocol CallServiceObserver: class {
         let sessionDescription = RTCSessionDescription(type: .answer, sdp: sessionDescription)
         _ = peerConnectionClient.setRemoteSessionDescription(sessionDescription).then {
             Logger.debug("\(self.TAG) successfully set remote description")
-        }.catch() { error in
+        }.catch { error in
             if let callError = error as? CallError {
                 self.handleFailedCall(error: callError)
             } else {
@@ -424,10 +425,11 @@ protocol CallServiceObserver: class {
 
         incomingCallPromise = firstly {
             return getIceServers()
-        }.then() { (iceServers: [RTCIceServer]) -> Promise<HardenedRTCSessionDescription> in
+        }.then { (iceServers: [RTCIceServer]) -> Promise<HardenedRTCSessionDescription> in
             // FIXME for first time call recipients I think we'll see mic/camera permission requests here,
             // even though, from the users perspective, no incoming call is yet visible.
             assert(self.peerConnectionClient == nil, "Unexpected PeerConnectionClient instance")
+            Logger.debug("\(self.self.TAG) setting peerConnectionClient in \(#function)")
             self.peerConnectionClient = PeerConnectionClient(iceServers: iceServers, delegate: self)
 
             let offerSessionDescription = RTCSessionDescription(type: .offer, sdp: callerSessionDescription)
@@ -435,14 +437,14 @@ protocol CallServiceObserver: class {
 
             // Find a sessionDescription compatible with my constraints and the remote sessionDescription
             return self.peerConnectionClient!.negotiateSessionDescription(remoteDescription: offerSessionDescription, constraints: constraints)
-        }.then() { (negotiatedSessionDescription: HardenedRTCSessionDescription) in
+        }.then { (negotiatedSessionDescription: HardenedRTCSessionDescription) in
             Logger.debug("\(self.TAG) set the remote description")
 
             let answerMessage = OWSCallAnswerMessage(callId: newCall.signalingId, sessionDescription: negotiatedSessionDescription.sdp)
             let callAnswerMessage = OWSOutgoingCallMessage(thread: thread, answerMessage: answerMessage)
 
             return self.messageSender.sendCallMessage(callAnswerMessage)
-        }.then() {
+        }.then {
             Logger.debug("\(self.TAG) successfully sent callAnswerMessage")
 
             let (promise, fulfill, _) = Promise<Void>.pending()
@@ -456,7 +458,7 @@ protocol CallServiceObserver: class {
             self.fulfillCallConnectedPromise = fulfill
 
             return race(promise, timeout)
-        }.catch() { error in
+        }.catch { error in
             if let callError = error as? CallError {
                 self.handleFailedCall(error: callError)
             } else {
@@ -771,9 +773,9 @@ protocol CallServiceObserver: class {
         // If the call hasn't started yet, we don't have a data channel to communicate the hang up. Use Signal Service Message.
         let hangupMessage = OWSCallHangupMessage(callId: call.signalingId)
         let callMessage = OWSOutgoingCallMessage(thread: thread, hangupMessage: hangupMessage)
-        _  = self.messageSender.sendCallMessage(callMessage).then() {
+        _  = self.messageSender.sendCallMessage(callMessage).then {
             Logger.debug("\(self.TAG) successfully sent hangup call message to \(thread)")
-        }.catch() { error in
+        }.catch { error in
             Logger.error("\(self.TAG) failed to send hangup call message to \(thread) with error: \(error)")
         }
 
@@ -921,18 +923,28 @@ protocol CallServiceObserver: class {
     /**
      * The connection has been established. The clients can now communicate.
      */
-    internal func peerConnectionClientIceConnected(_ peerconnectionClient: PeerConnectionClient) {
+    internal func peerConnectionClientIceConnected(_ peerConnectionClient: PeerConnectionClient) {
         AssertIsOnMainThread()
 
+        guard peerConnectionClient == self.peerConnectionClient else {
+            Logger.debug("\(self.TAG) \(#function) Ignoring event from obsolete peerConnectionClient")
+            return
+        }
+
         self.handleIceConnected()
     }
 
     /**
      * The connection failed to establish. The clients will not be able to communicate.
      */
-    internal func peerConnectionClientIceFailed(_ peerconnectionClient: PeerConnectionClient) {
+    internal func peerConnectionClientIceFailed(_ peerConnectionClient: PeerConnectionClient) {
         AssertIsOnMainThread()
 
+        guard peerConnectionClient == self.peerConnectionClient else {
+            Logger.debug("\(self.TAG) \(#function) Ignoring event from obsolete peerConnectionClient")
+            return
+        }
+
         self.handleFailedCall(error: CallError.disconnected)
     }
 
@@ -941,31 +953,51 @@ protocol CallServiceObserver: class {
      * reach the local client via the internet. The delegate must shuttle these IceCandates to the other (remote) client
      * out of band, as part of establishing a connection over WebRTC.
      */
-    internal func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, addedLocalIceCandidate iceCandidate: RTCIceCandidate) {
+    internal func peerConnectionClient(_ peerConnectionClient: PeerConnectionClient, addedLocalIceCandidate iceCandidate: RTCIceCandidate) {
         AssertIsOnMainThread()
 
+        guard peerConnectionClient == self.peerConnectionClient else {
+            Logger.debug("\(self.TAG) \(#function) Ignoring event from obsolete peerConnectionClient")
+            return
+        }
+
         self.handleLocalAddedIceCandidate(iceCandidate)
     }
 
     /**
      * Once the peerconnection is established, we can receive messages via the data channel, and notify the delegate.
      */
-    internal func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, received dataChannelMessage: OWSWebRTCProtosData) {
+    internal func peerConnectionClient(_ peerConnectionClient: PeerConnectionClient, received dataChannelMessage: OWSWebRTCProtosData) {
         AssertIsOnMainThread()
 
+        guard peerConnectionClient == self.peerConnectionClient else {
+            Logger.debug("\(self.TAG) \(#function) Ignoring event from obsolete peerConnectionClient")
+            return
+        }
+
         self.handleDataChannelMessage(dataChannelMessage)
     }
 
-    internal func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, didUpdateLocal videoTrack: RTCVideoTrack?) {
+    internal func peerConnectionClient(_ peerConnectionClient: PeerConnectionClient, didUpdateLocal videoTrack: RTCVideoTrack?) {
         AssertIsOnMainThread()
 
+        guard peerConnectionClient == self.peerConnectionClient else {
+            Logger.debug("\(self.TAG) \(#function) Ignoring event from obsolete peerConnectionClient")
+            return
+        }
+
         self.localVideoTrack = videoTrack
         self.fireDidUpdateVideoTracks()
     }
 
-    internal func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, didUpdateRemote videoTrack: RTCVideoTrack?) {
+    internal func peerConnectionClient(_ peerConnectionClient: PeerConnectionClient, didUpdateRemote videoTrack: RTCVideoTrack?) {
         AssertIsOnMainThread()
 
+        guard peerConnectionClient == self.peerConnectionClient else {
+            Logger.debug("\(self.TAG) \(#function) Ignoring event from obsolete peerConnectionClient")
+            return
+        }
+
         self.remoteVideoTrack = videoTrack
         self.fireDidUpdateVideoTracks()
     }
@@ -981,7 +1013,7 @@ protocol CallServiceObserver: class {
 
         return firstly {
             return accountManager.getTurnServerInfo()
-        }.then() { turnServerInfo -> [RTCIceServer] in
+        }.then { turnServerInfo -> [RTCIceServer] in
             Logger.debug("\(self.TAG) got turn server urls: \(turnServerInfo.urls)")
 
             return turnServerInfo.urls.map { url in
@@ -1028,21 +1060,21 @@ protocol CallServiceObserver: class {
 
         Logger.debug("\(TAG) in \(#function)")
 
-        PeerConnectionClient.stopAudioSession()
-        peerConnectionClient?.terminate()
-
-        peerConnectionClient = nil
         localVideoTrack = nil
         remoteVideoTrack = nil
         isRemoteVideoEnabled = false
+
+        PeerConnectionClient.stopAudioSession()
+        peerConnectionClient?.terminate()
+        Logger.debug("\(TAG) setting peerConnectionClient in \(#function)")
+        peerConnectionClient = nil
+
         call?.removeAllObservers()
         call = nil
         thread = nil
         incomingCallPromise = nil
         sendIceUpdatesImmediately = true
         pendingIceUpdateMessages = []
-
-        fireDidUpdateVideoTracks()
     }
 
     // MARK: - CallObserver

Signal/src/call/PeerConnectionClient.swift

@@ -423,10 +423,7 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD
         AssertIsOnMainThread()
         Logger.debug("\(TAG) in \(#function)")
 
-        delegate.peerConnectionClient(self, didUpdateLocal: nil)
+        PeerConnectionClient.signalingQueue.sync {
-        delegate.peerConnectionClient(self, didUpdateRemote: nil)
-
-        PeerConnectionClient.signalingQueue.async {
             assert(self.peerConnection != nil)
             self.terminateInternal()
         }