diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index 651fa288b..51ff18276 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -449,9 +449,8 @@ private class SignalCallData: NSObject { self.handleFailedCall(failedCall: call, error: .obsoleteCall(description:"obsolete call in \(#function)")) return } - guard callData.call == call else { - self.handleFailedCall(failedCall: call, error: .obsoleteCall(description:"obsolete call in \(#function)")) + Logger.warn("\(self.logTag) ignoring \(#function) for call other than current call") return } @@ -746,11 +745,11 @@ private class SignalCallData: NSObject { * Remote client (could be caller or callee) sent us a connectivity update */ public func handleRemoteAddedIceCandidate(thread: TSContactThread, callId: UInt64, sdp: String, lineIndex: Int32, mid: String) { + SwiftAssertIsOnMainThread(#function) Logger.verbose("\(logTag) \(#function) callId: \(callId)") guard let callData = self.callData else { - OWSProdError(OWSAnalyticsEvents.callServiceCallMissing(), file: #file, function: #function, line: #line) - self.handleFailedCurrentCall(error: .obsoleteCall(description: "ignoring remote ice update, since there is no current call.")) + Logger.info("\(logTag) ignoring remote ice update, since there is no current call.") return } @@ -1002,7 +1001,7 @@ private class SignalCallData: NSObject { Logger.info("\(self.logTag) in \(#function)") SwiftAssertIsOnMainThread(#function) - guard let peerConnectionClient = self.peerConnectionClient else { + guard let peerConnectionClient = callData.peerConnectionClient else { OWSProdError(OWSAnalyticsEvents.callServicePeerConnectionMissing(), file: #file, function: #function, line: #line) handleFailedCall(failedCall: call, error: CallError.assertionError(description: "\(self.logTag) peerConnectionClient unexpectedly nil in \(#function)")) return diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift index 2a03883b4..61da0048c 100644 --- a/Signal/src/call/PeerConnectionClient.swift +++ b/Signal/src/call/PeerConnectionClient.swift @@ -74,7 +74,54 @@ protocol PeerConnectionClientDelegate: class { */ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelDelegate { - let TAG = "[PeerConnectionClient]" + // In Swift (at least in Swift v3.3), weak variables aren't thread safe. It + // isn't safe to resolve/acquire/lock a weak reference into a strong reference + // while the instance might be being deallocated on another thread. + // + // AtomicHandle provides thread-safe access to a strong reference. + // PeerConnectionClient has an AtomicHandle to itself that its many async blocks + // (which run on more than one thread) can use to safely try to acquire a strong + // reference to the PeerConnectionClient. In ARC we'd normally, we'd avoid + // having an instance retain a strong reference to itself to avoid retain + // cycles, but it's safe in this case: PeerConnectionClient is owned (and only + // used by) a single entity CallService and CallService always calls + // [PeerConnectionClient terminate] when it is done with a PeerConnectionClient + // instance, so terminate is a reliable place where we can break the retain cycle. + // + // TODO: This should be fixed in Swift 4. To verify, remove AtomicHandle and + // use [weak self] as usual. Test using the following scenarios: + // + // * Alice and Bob place simultaneous calls to each other. Both should get busy. + // Repeat 10-20x. Then verify that they can connect a call by having just one + // call the other. + // * Alice or Bob (randomly alternating) calls the other. Recipient (randomly) + // accepts call or hangs up. If accepted, Alice or Bob (randomly) hangs up. + // Repeat immediately, as fast as you can, 10-20x. + private class AtomicHandle : NSObject { + var value: ValueType? + + func set(value: ValueType) { + objc_sync_enter(self) + self.value = value + objc_sync_exit(self) + } + + func get() -> ValueType? { + objc_sync_enter(self) + let result = value + objc_sync_exit(self) + return result + } + + func clear() { + Logger.info("\(logTag) \(#function)") + + objc_sync_enter(self) + value = nil + objc_sync_exit(self) + } + } + enum Identifiers: String { case mediaStream = "ARDAMS", videoTrack = "ARDAMSv0", @@ -95,7 +142,7 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD // Connection - private var peerConnection: RTCPeerConnection! + private var peerConnection: RTCPeerConnection? private let iceServers: [RTCIceServer] private let connectionConstraints: RTCMediaConstraints private let configuration: RTCConfiguration @@ -121,13 +168,11 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD // RTCVideoTrack is fragile and prone to throwing exceptions and/or // causing deadlock in its destructor. Therefore we take great care // with this property. - // - // We synchronize access to this property and ensure that we never - // set or use a strong reference to the remote video track if - // peerConnection is nil. private var remoteVideoTrack: RTCVideoTrack? private var cameraConstraints: RTCMediaConstraints + private let handle: AtomicHandle + init(iceServers: [RTCIceServer], delegate: PeerConnectionClientDelegate, callDirection: CallDirection, useTurnOnly: Bool) { SwiftAssertIsOnMainThread(#function) @@ -139,10 +184,10 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD configuration.bundlePolicy = .maxBundle configuration.rtcpMuxPolicy = .require if useTurnOnly { - Logger.debug("\(TAG) using iceTransportPolicy: relay") + Logger.debug("\(PeerConnectionClient.logTag) using iceTransportPolicy: relay") configuration.iceTransportPolicy = .relay } else { - Logger.debug("\(TAG) using iceTransportPolicy: default") + Logger.debug("\(PeerConnectionClient.logTag) using iceTransportPolicy: default") } let connectionConstraintsDict = ["DtlsSrtpKeyAgreement": "true"] @@ -151,8 +196,12 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD audioConstraints = RTCMediaConstraints(mandatoryConstraints: nil, optionalConstraints: nil) cameraConstraints = RTCMediaConstraints(mandatoryConstraints: nil, optionalConstraints: nil) + handle = AtomicHandle() + super.init() + handle.set(value: self) + peerConnection = factory.peerConnection(with: configuration, constraints: connectionConstraints, delegate: self) @@ -167,13 +216,19 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD } deinit { - Logger.debug("[PeerConnectionClient] deinit") + // TODO: We can demote this log level to debug once we're confident that + // this class is always deallocated. + Logger.info("[PeerConnectionClient] deinit") } // MARK: - Media Streams private func createSignalingDataChannel() { SwiftAssertIsOnMainThread(#function) + guard let peerConnection = peerConnection else { + Logger.debug("\(logTag) \(#function) Ignoring obsolete event in terminated client") + return + } let configuration = RTCDataChannelConfiguration() // Insist upon an "ordered" TCP data channel for delivery reliability. @@ -190,12 +245,15 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD fileprivate func createVideoSender() { SwiftAssertIsOnMainThread(#function) - - Logger.debug("\(TAG) in \(#function)") + Logger.debug("\(logTag) in \(#function)") assert(self.videoSender == nil, "\(#function) should only be called once.") guard !Platform.isSimulator else { - Logger.warn("\(TAG) Refusing to create local video track on simulator which has no capture device.") + Logger.warn("\(logTag) Refusing to create local video track on simulator which has no capture device.") + return + } + guard let peerConnection = peerConnection else { + Logger.debug("\(logTag) \(#function) Ignoring obsolete event in terminated client") return } @@ -227,15 +285,17 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD public func setCameraSource(useBackCamera: Bool) { SwiftAssertIsOnMainThread(#function) - PeerConnectionClient.signalingQueue.async { - guard let localVideoSource = self.localVideoSource else { - owsFail("\(self.logTag) in \(#function) localVideoSource was unexpectedly nil") + let handle = self.handle + PeerConnectionClient.signalingQueue.async { [weak self] in + guard let strongSelf = handle.get() else { return } + guard let localVideoSource = strongSelf.localVideoSource else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") return } // certain devices, e.g. 16GB iPod touch don't have a back camera guard localVideoSource.canUseBackCamera else { - owsFail("\(self.logTag) in \(#function) canUseBackCamera was unexpectedly false") + owsFail("\(strongSelf.logTag) in \(#function) canUseBackCamera was unexpectedly false") return } @@ -245,38 +305,40 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD public func setLocalVideoEnabled(enabled: Bool) { SwiftAssertIsOnMainThread(#function) + let handle = self.handle + let completion = { [weak self] in + guard let strongSelf = handle.get() else { return } + guard let localVideoTrack = strongSelf.localVideoTrack else { return } + guard let strongDelegate = strongSelf.delegate else { return } + strongDelegate.peerConnectionClient(strongSelf, didUpdateLocal: enabled ? localVideoTrack : nil) + } - PeerConnectionClient.signalingQueue.async { - guard self.peerConnection != nil else { - Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") + PeerConnectionClient.signalingQueue.async { [weak self] in + guard let strongSelf = handle.get() else { return } + guard strongSelf.peerConnection != nil else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") return } - guard let localVideoTrack = self.localVideoTrack else { - let action = enabled ? "enable" : "disable" - Logger.error("\(self.TAG)) trying to \(action) videoTrack which doesn't exist") + guard let localVideoTrack = strongSelf.localVideoTrack else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") return } - guard let videoCaptureSession = self.videoCaptureSession else { - Logger.debug("\(self.TAG) videoCaptureSession was unexpectedly nil") + guard let videoCaptureSession = strongSelf.videoCaptureSession else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") return } localVideoTrack.isEnabled = enabled if enabled { - Logger.debug("\(self.TAG) in \(#function) starting videoCaptureSession") + Logger.debug("\(strongSelf.logTag) in \(#function) starting videoCaptureSession") videoCaptureSession.startRunning() } else { - Logger.debug("\(self.TAG) in \(#function) stopping videoCaptureSession") + Logger.debug("\(strongSelf.logTag) in \(#function) stopping videoCaptureSession") videoCaptureSession.stopRunning() } - DispatchQueue.main.async { [weak self, weak localVideoTrack] in - guard let strongSelf = self else { return } - guard let strongLocalVideoTrack = localVideoTrack else { return } - guard let strongDelegate = strongSelf.delegate else { return } - strongDelegate.peerConnectionClient(strongSelf, didUpdateLocal: enabled ? strongLocalVideoTrack : nil) - } + DispatchQueue.main.async(execute: completion) } } @@ -284,10 +346,14 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD fileprivate func createAudioSender() { SwiftAssertIsOnMainThread(#function) - - Logger.debug("\(TAG) in \(#function)") + Logger.debug("\(logTag) in \(#function)") assert(self.audioSender == nil, "\(#function) should only be called once.") + guard let peerConnection = peerConnection else { + Logger.debug("\(logTag) \(#function) Ignoring obsolete event in terminated client") + return + } + let audioSource = factory.audioSource(with: self.audioConstraints) let audioTrack = factory.audioTrack(with: audioSource, trackId: Identifiers.audioTrack.rawValue) @@ -306,15 +372,15 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD public func setAudioEnabled(enabled: Bool) { SwiftAssertIsOnMainThread(#function) - - PeerConnectionClient.signalingQueue.async { - guard self.peerConnection != nil else { - Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") + let handle = self.handle + PeerConnectionClient.signalingQueue.async { [weak self] in + guard let strongSelf = handle.get() else { return } + guard strongSelf.peerConnection != nil else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") return } - guard let audioTrack = self.audioTrack else { - let action = enabled ? "enable" : "disable" - Logger.error("\(self.TAG) trying to \(action) audioTrack which doesn't exist.") + guard let audioTrack = strongSelf.audioTrack else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") return } @@ -334,181 +400,249 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD public func createOffer() -> Promise { SwiftAssertIsOnMainThread(#function) + let handle = self.handle + let (promise, fulfill, reject) = Promise.pending() + let completion: ((RTCSessionDescription?, Error?) -> Void) = { [weak self] (sdp, error) in + guard let strongSelf = handle.get() else { + reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) + return + } + strongSelf.assertOnSignalingQueue() + guard strongSelf.peerConnection != nil else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") + reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) + return + } + if let error = error { + reject(error) + return + } - return Promise { fulfill, reject in - SwiftAssertIsOnMainThread(#function) - - PeerConnectionClient.signalingQueue.async { - guard self.peerConnection != nil else { - Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") - reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) - return - } - - self.peerConnection.offer(for: self.defaultOfferConstraints, completionHandler: { (sdp: RTCSessionDescription?, error: Error?) in - PeerConnectionClient.signalingQueue.async { - guard self.peerConnection != nil else { - Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") - reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) - return - } - guard error == nil else { - reject(error!) - return - } + guard let sessionDescription = sdp else { + Logger.error("\(strongSelf.logTag) No session description was obtained, even though there was no error reported.") + let error = OWSErrorMakeUnableToProcessServerResponseError() + reject(error) + return + } - guard let sessionDescription = sdp else { - Logger.error("\(self.TAG) No session description was obtained, even though there was no error reported.") - let error = OWSErrorMakeUnableToProcessServerResponseError() - reject(error) - return - } + fulfill(HardenedRTCSessionDescription(rtcSessionDescription: sessionDescription)) + } - fulfill(HardenedRTCSessionDescription(rtcSessionDescription: sessionDescription)) - } - }) + PeerConnectionClient.signalingQueue.async { [weak self] in + guard let strongSelf = handle.get() else { + reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) + return + } + strongSelf.assertOnSignalingQueue() + guard let peerConnection = strongSelf.peerConnection else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") + reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) + return } + + peerConnection.offer(for: strongSelf.defaultOfferConstraints, completionHandler: { (sdp: RTCSessionDescription?, error: Error?) in + PeerConnectionClient.signalingQueue.async { + completion(sdp, error) + } + }) } + + return promise } public func setLocalSessionDescriptionInternal(_ sessionDescription: HardenedRTCSessionDescription) -> Promise { - return PromiseKit.wrap { resolve in - self.assertOnSignalingQueue() - Logger.verbose("\(self.TAG) setting local session description: \(sessionDescription)") - self.peerConnection.setLocalDescription(sessionDescription.rtcSessionDescription, completionHandler: resolve) + let handle = self.handle + let (promise, fulfill, reject) = Promise.pending() + PeerConnectionClient.signalingQueue.async { [weak self] in + guard let strongSelf = handle.get() else { + reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) + return + } + strongSelf.assertOnSignalingQueue() + + guard let peerConnection = strongSelf.peerConnection else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") + reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) + return + } + + Logger.verbose("\(strongSelf.logTag) setting local session description: \(sessionDescription)") + peerConnection.setLocalDescription(sessionDescription.rtcSessionDescription, completionHandler: { (error) in + if let error = error { + reject(error) + } else { + fulfill() + } + }) } + return promise } public func setLocalSessionDescription(_ sessionDescription: HardenedRTCSessionDescription) -> Promise { SwiftAssertIsOnMainThread(#function) - - return Promise { fulfill, reject in - PeerConnectionClient.signalingQueue.async { - guard self.peerConnection != nil else { - Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") - reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) - return - } - Logger.verbose("\(self.TAG) setting local session description: \(sessionDescription)") - self.peerConnection.setLocalDescription(sessionDescription.rtcSessionDescription, - completionHandler: { error in - guard error == nil else { - reject(error!) - return - } - fulfill() - }) + let handle = self.handle + let (promise, fulfill, reject) = Promise.pending() + PeerConnectionClient.signalingQueue.async { [weak self] in + guard let strongSelf = handle.get() else { + reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) + return } + strongSelf.assertOnSignalingQueue() + guard let peerConnection = strongSelf.peerConnection else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") + reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) + return + } + + Logger.verbose("\(strongSelf.logTag) setting local session description: \(sessionDescription)") + peerConnection.setLocalDescription(sessionDescription.rtcSessionDescription, + completionHandler: { error in + if let error = error { + reject(error) + return + } + fulfill() + }) } + + return promise } public func negotiateSessionDescription(remoteDescription: RTCSessionDescription, constraints: RTCMediaConstraints) -> Promise { SwiftAssertIsOnMainThread(#function) - + let handle = self.handle return setRemoteSessionDescription(remoteDescription) - .then(on: PeerConnectionClient.signalingQueue) { - return self.negotiateAnswerSessionDescription(constraints: constraints) + .then(on: PeerConnectionClient.signalingQueue) { [weak self] in + guard let strongSelf = handle.get() else { + return Promise(error: NSError(domain: "Obsolete client", code: 0, userInfo: nil)) + } + return strongSelf.negotiateAnswerSessionDescription(constraints: constraints) } } public func setRemoteSessionDescription(_ sessionDescription: RTCSessionDescription) -> Promise { SwiftAssertIsOnMainThread(#function) - - return Promise { fulfill, reject in - PeerConnectionClient.signalingQueue.async { - guard self.peerConnection != nil else { - Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") - reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) - return - } - Logger.verbose("\(self.TAG) setting remote description: \(sessionDescription)") - self.peerConnection.setRemoteDescription(sessionDescription, - completionHandler: { error in - guard error == nil else { - reject(error!) - return - } - fulfill() - }) + let handle = self.handle + let (promise, fulfill, reject) = Promise.pending() + PeerConnectionClient.signalingQueue.async { [weak self] in + guard let strongSelf = handle.get() else { + reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) + return + } + strongSelf.assertOnSignalingQueue() + guard let peerConnection = strongSelf.peerConnection else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") + reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) + return } + Logger.verbose("\(strongSelf.logTag) setting remote description: \(sessionDescription)") + peerConnection.setRemoteDescription(sessionDescription, + completionHandler: { error in + if let error = error { + reject(error) + return + } + fulfill() + }) } + return promise } private func negotiateAnswerSessionDescription(constraints: RTCMediaConstraints) -> Promise { assertOnSignalingQueue() + let handle = self.handle + let (promise, fulfill, reject) = Promise.pending() + let completion: ((RTCSessionDescription?, Error?) -> Void) = { [weak self] (sdp, error) in + guard let strongSelf = handle.get() else { + reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) + return + } + strongSelf.assertOnSignalingQueue() + guard strongSelf.peerConnection != nil else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") + reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) + return + } + if let error = error { + reject(error) + return + } + + guard let sessionDescription = sdp else { + Logger.error("\(strongSelf.logTag) unexpected empty session description, even though no error was reported.") + let error = OWSErrorMakeUnableToProcessServerResponseError() + reject(error) + return + } + + let hardenedSessionDescription = HardenedRTCSessionDescription(rtcSessionDescription: sessionDescription) + + strongSelf.setLocalSessionDescriptionInternal(hardenedSessionDescription) + .then(on: PeerConnectionClient.signalingQueue) { _ in + fulfill(hardenedSessionDescription) + }.catch { error in + reject(error) + } + } - return Promise { fulfill, reject in - assertOnSignalingQueue() + PeerConnectionClient.signalingQueue.async { [weak self] in + guard let strongSelf = handle.get() else { + reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) + return + } + strongSelf.assertOnSignalingQueue() - guard self.peerConnection != nil else { - Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") + guard let peerConnection = strongSelf.peerConnection else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) return } - Logger.debug("\(self.TAG) negotiating answer session.") + Logger.debug("\(strongSelf.logTag) negotiating answer session.") peerConnection.answer(for: constraints, completionHandler: { (sdp: RTCSessionDescription?, error: Error?) in PeerConnectionClient.signalingQueue.async { - guard self.peerConnection != nil else { - Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") - reject(NSError(domain: "Obsolete client", code: 0, userInfo: nil)) - return - } - guard error == nil else { - reject(error!) - return - } - - guard let sessionDescription = sdp else { - Logger.error("\(self.TAG) unexpected empty session description, even though no error was reported.") - let error = OWSErrorMakeUnableToProcessServerResponseError() - reject(error) - return - } - - let hardenedSessionDescription = HardenedRTCSessionDescription(rtcSessionDescription: sessionDescription) - - self.setLocalSessionDescriptionInternal(hardenedSessionDescription) - .then(on: PeerConnectionClient.signalingQueue) { - fulfill(hardenedSessionDescription) - }.catch { error in - reject(error) - } + completion(sdp, error) } }) } + return promise } public func addRemoteIceCandidate(_ candidate: RTCIceCandidate) { - PeerConnectionClient.signalingQueue.async { - guard self.peerConnection != nil else { - Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") + let handle = self.handle + PeerConnectionClient.signalingQueue.async { [weak self] in + guard let strongSelf = handle.get() else { return } + guard let peerConnection = strongSelf.peerConnection else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") return } - Logger.info("\(self.TAG) adding remote ICE candidate: \(candidate.sdp)") - self.peerConnection.add(candidate) + Logger.info("\(strongSelf.logTag) adding remote ICE candidate: \(candidate.sdp)") + peerConnection.add(candidate) } } public func terminate() { SwiftAssertIsOnMainThread(#function) - Logger.debug("\(TAG) in \(#function)") + Logger.debug("\(logTag) in \(#function)") // Clear the delegate immediately so that we can guarantee that // no delegate methods are called after terminate() returns. delegate = nil + // Clear the handle immediately so that enqueued work is aborted + // going forward. + handle.clear() + + // Don't use [weak self]; we always want to perform terminateInternal(). PeerConnectionClient.signalingQueue.async { - assert(self.peerConnection != nil) self.terminateInternal() } } private func terminateInternal() { assertOnSignalingQueue() - - Logger.debug("\(TAG) in \(#function)") + Logger.debug("\(logTag) in \(#function)") // Some notes on preventing crashes while disposing of peerConnection for video calls // from: https://groups.google.com/forum/#!searchin/discuss-webrtc/objc$20crash$20dealloc%7Csort:relevance/discuss-webrtc/7D-vk5yLjn8/rBW2D6EW4GYJ @@ -523,12 +657,13 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD // become nil when it was only a weak property. So we retain it and manually nil the reference here, because // we are likely to crash if we retain any peer connection properties when the peerconnection is released - // See the comments on the remoteVideoTrack property. - objc_sync_enter(self) - localVideoTrack?.isEnabled = false remoteVideoTrack?.isEnabled = false + if let dataChannel = self.dataChannel { + dataChannel.delegate = nil + } + dataChannel = nil audioSender = nil audioTrack = nil @@ -537,11 +672,11 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD localVideoTrack = nil remoteVideoTrack = nil - peerConnection.delegate = nil - peerConnection.close() + if let peerConnection = peerConnection { + peerConnection.delegate = nil + peerConnection.close() + } peerConnection = nil - - objc_sync_exit(self) } // MARK: - Data Channel @@ -556,32 +691,34 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD public func sendDataChannelMessage(data: Data, description: String, isCritical: Bool) { SwiftAssertIsOnMainThread(#function) + let handle = self.handle + PeerConnectionClient.signalingQueue.async { [weak self] in + guard let strongSelf = handle.get() else { return } - PeerConnectionClient.signalingQueue.async { - guard self.peerConnection != nil else { - Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client: \(description)") + guard strongSelf.peerConnection != nil else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client: \(description)") return } - guard let dataChannel = self.dataChannel else { + guard let dataChannel = strongSelf.dataChannel else { if isCritical { - Logger.info("\(self.TAG) in \(#function) enqueuing critical data channel message for after we have a dataChannel: \(description)") - self.pendingDataChannelMessages.append(PendingDataChannelMessage(data: data, description: description, isCritical: isCritical)) + Logger.info("\(strongSelf.logTag) in \(#function) enqueuing critical data channel message for after we have a dataChannel: \(description)") + strongSelf.pendingDataChannelMessages.append(PendingDataChannelMessage(data: data, description: description, isCritical: isCritical)) } else { - Logger.error("\(self.TAG) in \(#function) ignoring sending \(data) for nil dataChannel: \(description)") + Logger.error("\(strongSelf.logTag) in \(#function) ignoring sending \(data) for nil dataChannel: \(description)") } return } - Logger.debug("\(self.TAG) sendDataChannelMessage trying: \(description)") + Logger.debug("\(strongSelf.logTag) sendDataChannelMessage trying: \(description)") let buffer = RTCDataBuffer(data: data, isBinary: false) let result = dataChannel.sendData(buffer) if result { - Logger.debug("\(self.TAG) sendDataChannelMessage succeeded: \(description)") + Logger.debug("\(strongSelf.logTag) sendDataChannelMessage succeeded: \(description)") } else { - Logger.warn("\(self.TAG) sendDataChannelMessage failed: \(description)") + Logger.warn("\(strongSelf.logTag) sendDataChannelMessage failed: \(description)") if isCritical { OWSProdError(OWSAnalyticsEvents.peerConnectionClientErrorSendDataChannelMessageFailed(), file: #file, function: #function, line: #line) } @@ -593,177 +730,217 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD /** The data channel state changed. */ internal func dataChannelDidChangeState(_ dataChannel: RTCDataChannel) { - Logger.debug("\(TAG) dataChannelDidChangeState: \(dataChannel)") + Logger.debug("\(logTag) dataChannelDidChangeState: \(dataChannel)") } /** The data channel successfully received a data buffer. */ internal func dataChannel(_ dataChannel: RTCDataChannel, didReceiveMessageWith buffer: RTCDataBuffer) { - PeerConnectionClient.signalingQueue.async { - guard self.peerConnection != nil else { - Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") + let handle = self.handle + let completion: (OWSWebRTCProtosData) -> Void = { [weak self] (dataChannelMessage) in + SwiftAssertIsOnMainThread(#function) + guard let strongSelf = handle.get() else { return } + guard let strongDelegate = strongSelf.delegate else { return } + strongDelegate.peerConnectionClient(strongSelf, received: dataChannelMessage) + } + + PeerConnectionClient.signalingQueue.async { [weak self] in + guard let strongSelf = handle.get() else { return } + guard strongSelf.peerConnection != nil else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") return } - Logger.debug("\(self.TAG) dataChannel didReceiveMessageWith buffer:\(buffer)") + Logger.debug("\(strongSelf.logTag) dataChannel didReceiveMessageWith buffer:\(buffer)") guard let dataChannelMessage = OWSWebRTCProtosData.parse(from: buffer.data) else { // TODO can't proto parsings throw an exception? Is it just being lost in the Objc->Swift? - Logger.error("\(self.TAG) failed to parse dataProto") + Logger.error("\(strongSelf.logTag) failed to parse dataProto") return } - DispatchQueue.main.async { [weak self] in - guard let strongSelf = self else { return } - guard let strongDelegate = strongSelf.delegate else { return } - strongDelegate.peerConnectionClient(strongSelf, received: dataChannelMessage) + DispatchQueue.main.async { + completion(dataChannelMessage) } } } /** The data channel's |bufferedAmount| changed. */ internal func dataChannel(_ dataChannel: RTCDataChannel, didChangeBufferedAmount amount: UInt64) { - Logger.debug("\(TAG) didChangeBufferedAmount: \(amount)") + Logger.debug("\(logTag) didChangeBufferedAmount: \(amount)") } // MARK: - RTCPeerConnectionDelegate /** Called when the SignalingState changed. */ - internal func peerConnection(_ peerConnection: RTCPeerConnection, didChange stateChanged: RTCSignalingState) { - Logger.debug("\(TAG) didChange signalingState:\(stateChanged.debugDescription)") + internal func peerConnection(_ peerConnectionParam: RTCPeerConnection, didChange stateChanged: RTCSignalingState) { + Logger.debug("\(logTag) didChange signalingState:\(stateChanged.debugDescription)") } /** Called when media is received on a new stream from remote peer. */ - internal func peerConnection(_ peerConnection: RTCPeerConnection, didAdd stream: RTCMediaStream) { - guard stream.videoTracks.count > 0 else { - return - } - let remoteVideoTrack = stream.videoTracks[0] - Logger.debug("\(self.TAG) didAdd stream:\(stream) video tracks: \(stream.videoTracks.count) audio tracks: \(stream.audioTracks.count)") + internal func peerConnection(_ peerConnectionParam: RTCPeerConnection, didAdd stream: RTCMediaStream) { + let handle = self.handle + let completion: (RTCVideoTrack) -> Void = { [weak self] (remoteVideoTrack) in + SwiftAssertIsOnMainThread(#function) + guard let strongSelf = handle.get() else { return } + guard let strongDelegate = strongSelf.delegate else { return } - // See the comments on the remoteVideoTrack property. - // - // We only set the remoteVideoTrack property if peerConnection is non-nil. - objc_sync_enter(self) - if self.peerConnection != nil { - self.remoteVideoTrack = remoteVideoTrack + // TODO: Consider checking for termination here. + + strongDelegate.peerConnectionClient(strongSelf, didUpdateRemote: remoteVideoTrack) } - objc_sync_exit(self) - PeerConnectionClient.signalingQueue.async { - guard self.peerConnection != nil else { - Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") + PeerConnectionClient.signalingQueue.async { [weak self] in + guard let strongSelf = handle.get() else { return } + guard let peerConnection = strongSelf.peerConnection else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") return } + guard peerConnection == peerConnectionParam else { + owsFail("\(strongSelf.logTag) in \(#function) mismatched peerConnection callback.") + return + } + guard stream.videoTracks.count > 0 else { + owsFail("\(strongSelf.logTag) in \(#function) didAdd stream missing stream.") + return + } + let remoteVideoTrack = stream.videoTracks[0] + Logger.debug("\(strongSelf.logTag) didAdd stream:\(stream) video tracks: \(stream.videoTracks.count) audio tracks: \(stream.audioTracks.count)") - DispatchQueue.main.async { [weak self] in - guard let strongSelf = self else { return } - guard let strongDelegate = strongSelf.delegate else { return } - - // See the comments on the remoteVideoTrack property. - // - // We only access the remoteVideoTrack property if peerConnection is non-nil. - var remoteVideoTrack: RTCVideoTrack? - objc_sync_enter(strongSelf) - if strongSelf.peerConnection != nil { - remoteVideoTrack = strongSelf.remoteVideoTrack - } - objc_sync_exit(strongSelf) + strongSelf.remoteVideoTrack = remoteVideoTrack - strongDelegate.peerConnectionClient(strongSelf, didUpdateRemote: remoteVideoTrack) + DispatchQueue.main.async { + completion(remoteVideoTrack) } } } /** Called when a remote peer closes a stream. */ - internal func peerConnection(_ peerConnection: RTCPeerConnection, didRemove stream: RTCMediaStream) { - Logger.debug("\(TAG) didRemove Stream:\(stream)") + internal func peerConnection(_ peerConnectionParam: RTCPeerConnection, didRemove stream: RTCMediaStream) { + Logger.debug("\(logTag) didRemove Stream:\(stream)") } /** Called when negotiation is needed, for example ICE has restarted. */ - internal func peerConnectionShouldNegotiate(_ peerConnection: RTCPeerConnection) { - Logger.debug("\(TAG) shouldNegotiate") + internal func peerConnectionShouldNegotiate(_ peerConnectionParam: RTCPeerConnection) { + Logger.debug("\(logTag) shouldNegotiate") } /** Called any time the IceConnectionState changes. */ - internal func peerConnection(_ peerConnection: RTCPeerConnection, didChange newState: RTCIceConnectionState) { - PeerConnectionClient.signalingQueue.async { - guard self.peerConnection != nil else { - Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") + internal func peerConnection(_ peerConnectionParam: RTCPeerConnection, didChange newState: RTCIceConnectionState) { + let handle = self.handle + let connectedCompletion : () -> Void = { [weak self] in + SwiftAssertIsOnMainThread(#function) + guard let strongSelf = handle.get() else { return } + guard let strongDelegate = strongSelf.delegate else { return } + strongDelegate.peerConnectionClientIceConnected(strongSelf) + } + let failedCompletion : () -> Void = { [weak self] in + SwiftAssertIsOnMainThread(#function) + guard let strongSelf = handle.get() else { return } + guard let strongDelegate = strongSelf.delegate else { return } + strongDelegate.peerConnectionClientIceFailed(strongSelf) + } + let disconnectedCompletion : () -> Void = { [weak self] in + SwiftAssertIsOnMainThread(#function) + guard let strongSelf = handle.get() else { return } + guard let strongDelegate = strongSelf.delegate else { return } + strongDelegate.peerConnectionClientIceDisconnected(strongSelf) + } + + PeerConnectionClient.signalingQueue.async { [weak self] in + guard let strongSelf = handle.get() else { return } + guard let peerConnection = strongSelf.peerConnection else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") return } - Logger.info("\(self.TAG) didChange IceConnectionState:\(newState.debugDescription)") + guard peerConnection == peerConnectionParam else { + owsFail("\(strongSelf.logTag) in \(#function) mismatched peerConnection callback.") + return + } + + Logger.info("\(strongSelf.logTag) didChange IceConnectionState:\(newState.debugDescription)") switch newState { case .connected, .completed: - DispatchQueue.main.async { [weak self] in - guard let strongSelf = self else { return } - guard let strongDelegate = strongSelf.delegate else { return } - strongDelegate.peerConnectionClientIceConnected(strongSelf) - } + DispatchQueue.main.async(execute: connectedCompletion) case .failed: - Logger.warn("\(self.TAG) RTCIceConnection failed.") - DispatchQueue.main.async { [weak self] in - guard let strongSelf = self else { return } - guard let strongDelegate = strongSelf.delegate else { return } - strongDelegate.peerConnectionClientIceFailed(strongSelf) - } + Logger.warn("\(strongSelf.logTag) RTCIceConnection failed.") + DispatchQueue.main.async(execute: failedCompletion) case .disconnected: - Logger.warn("\(self.TAG) RTCIceConnection disconnected.") - DispatchQueue.main.async { [weak self] in - guard let strongSelf = self else { return } - guard let strongDelegate = strongSelf.delegate else { return } - strongDelegate.peerConnectionClientIceDisconnected(strongSelf) - } + Logger.warn("\(strongSelf.logTag) RTCIceConnection disconnected.") + DispatchQueue.main.async(execute: disconnectedCompletion) default: - Logger.debug("\(self.TAG) ignoring change IceConnectionState:\(newState.debugDescription)") + Logger.debug("\(strongSelf.logTag) ignoring change IceConnectionState:\(newState.debugDescription)") } } } /** Called any time the IceGatheringState changes. */ - internal func peerConnection(_ peerConnection: RTCPeerConnection, didChange newState: RTCIceGatheringState) { - Logger.info("\(TAG) didChange IceGatheringState:\(newState.debugDescription)") + internal func peerConnection(_ peerConnectionParam: RTCPeerConnection, didChange newState: RTCIceGatheringState) { + Logger.info("\(logTag) didChange IceGatheringState:\(newState.debugDescription)") } /** New ice candidate has been found. */ - internal func peerConnection(_ peerConnection: RTCPeerConnection, didGenerate candidate: RTCIceCandidate) { - PeerConnectionClient.signalingQueue.async { - guard self.peerConnection != nil else { - Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") + internal func peerConnection(_ peerConnectionParam: RTCPeerConnection, didGenerate candidate: RTCIceCandidate) { + let handle = self.handle + let completion: (RTCIceCandidate) -> Void = { [weak self] (candidate) in + SwiftAssertIsOnMainThread(#function) + guard let strongSelf = handle.get() else { return } + guard let strongDelegate = strongSelf.delegate else { return } + strongDelegate.peerConnectionClient(strongSelf, addedLocalIceCandidate: candidate) + } + + PeerConnectionClient.signalingQueue.async { [weak self] in + guard let strongSelf = handle.get() else { return } + guard let peerConnection = strongSelf.peerConnection else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") return } - Logger.info("\(self.TAG) adding local ICE candidate:\(candidate.sdp)") - DispatchQueue.main.async { [weak self] in - guard let strongSelf = self else { return } - guard let strongDelegate = strongSelf.delegate else { return } - strongDelegate.peerConnectionClient(strongSelf, addedLocalIceCandidate: candidate) + guard peerConnection == peerConnectionParam else { + owsFail("\(strongSelf.logTag) in \(#function) mismatched peerConnection callback.") + return + } + Logger.info("\(strongSelf.logTag) adding local ICE candidate:\(candidate.sdp)") + DispatchQueue.main.async { + completion(candidate) } } } /** Called when a group of local Ice candidates have been removed. */ - internal func peerConnection(_ peerConnection: RTCPeerConnection, didRemove candidates: [RTCIceCandidate]) { - Logger.debug("\(TAG) didRemove IceCandidates:\(candidates)") + internal func peerConnection(_ peerConnectionParam: RTCPeerConnection, didRemove candidates: [RTCIceCandidate]) { + Logger.debug("\(logTag) didRemove IceCandidates:\(candidates)") } /** New data channel has been opened. */ - internal func peerConnection(_ peerConnection: RTCPeerConnection, didOpen dataChannel: RTCDataChannel) { - PeerConnectionClient.signalingQueue.async { - guard self.peerConnection != nil else { - Logger.debug("\(self.TAG) \(#function) Ignoring obsolete event in terminated client") - return + internal func peerConnection(_ peerConnectionParam: RTCPeerConnection, didOpen dataChannel: RTCDataChannel) { + let handle = self.handle + let completion: ([PendingDataChannelMessage]) -> Void = { [weak self] (pendingMessages) in + SwiftAssertIsOnMainThread(#function) + guard let strongSelf = handle.get() else { return } + pendingMessages.forEach { message in + strongSelf.sendDataChannelMessage(data: message.data, description: message.description, isCritical: message.isCritical) } - Logger.info("\(self.TAG) didOpen dataChannel:\(dataChannel)") - assert(self.dataChannel == nil) - self.dataChannel = dataChannel - dataChannel.delegate = self + } - let pendingMessages = self.pendingDataChannelMessages - self.pendingDataChannelMessages = [] - DispatchQueue.main.async { [weak self] in - guard let strongSelf = self else { return } + PeerConnectionClient.signalingQueue.async { [weak self] in + guard let strongSelf = handle.get() else { return } + guard let peerConnection = strongSelf.peerConnection else { + Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") + return + } + guard peerConnection == peerConnectionParam else { + owsFail("\(strongSelf.logTag) in \(#function) mismatched peerConnection callback.") + return + } + Logger.info("\(strongSelf.logTag) didOpen dataChannel:\(dataChannel)") + if strongSelf.dataChannel != nil { + owsFail("\(strongSelf.logTag) in \(#function) dataChannel unexpectedly set twice.") + } + strongSelf.dataChannel = dataChannel + dataChannel.delegate = strongSelf - pendingMessages.forEach { message in - strongSelf.sendDataChannelMessage(data: message.data, description: message.description, isCritical: message.isCritical) - } + let pendingMessages = strongSelf.pendingDataChannelMessages + strongSelf.pendingDataChannelMessages = [] + DispatchQueue.main.async { + completion(pendingMessages) } } } @@ -785,7 +962,7 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD var result: RTCPeerConnection? = nil PeerConnectionClient.signalingQueue.sync { result = peerConnection - Logger.info("\(self.TAG) called \(#function)") + Logger.info("\(self.logTag) called \(#function)") } return result! } @@ -796,7 +973,7 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD var result: RTCDataChannel? = nil PeerConnectionClient.signalingQueue.sync { result = dataChannel - Logger.info("\(self.TAG) called \(#function)") + Logger.info("\(self.logTag) called \(#function)") } return result! }