From d294557bdd5d7df2e623352df2f198f9fa2cf770 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 1 Feb 2017 10:24:23 -0500 Subject: [PATCH] Rework concurrency in the signaling logic. // FREEBIE --- Podfile | 3 +- Podfile.lock | 8 ++--- Signal/src/Signal-Bridging-Header.h | 1 + Signal/src/call/CallService.swift | 24 ++++++------- Signal/src/call/PeerConnectionClient.swift | 35 +++++++++++-------- .../translations/en.lproj/Localizable.strings | 3 ++ 6 files changed, 43 insertions(+), 31 deletions(-) diff --git a/Podfile b/Podfile index b934649d5..a095f7f57 100644 --- a/Podfile +++ b/Podfile @@ -5,7 +5,8 @@ target 'Signal' do pod 'SocketRocket', :git => 'https://github.com/facebook/SocketRocket.git' pod 'AxolotlKit', git: 'https://github.com/WhisperSystems/SignalProtocolKit.git' #pod 'AxolotlKit', path: '../SignalProtocolKit' - pod 'SignalServiceKit', git: 'https://github.com/WhisperSystems/SignalServiceKit.git', branch: 'mkirk/webrtc' + #pod 'SignalServiceKit', git: 'https://github.com/WhisperSystems/SignalServiceKit.git', branch: 'mkirk/webrtc' + pod 'SignalServiceKit', git: 'https://github.com/WhisperSystems/SignalServiceKit.git', branch: 'charlesmchen/webrtc/threadSafety' #pod 'SignalServiceKit', path: '../SignalServiceKit' pod 'OpenSSL' pod 'PastelogKit', '~> 1.3' diff --git a/Podfile.lock b/Podfile.lock index 73b29d158..8cd84ef2d 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -123,7 +123,7 @@ DEPENDENCIES: - PastelogKit (~> 1.3) - PureLayout - SCWaveformView (~> 1.0) - - SignalServiceKit (from `https://github.com/WhisperSystems/SignalServiceKit.git`, branch `mkirk/webrtc`) + - SignalServiceKit (from `https://github.com/WhisperSystems/SignalServiceKit.git`, branch `charlesmchen/webrtc/threadSafety`) - SocketRocket (from `https://github.com/facebook/SocketRocket.git`) - ZXingObjC @@ -131,7 +131,7 @@ EXTERNAL SOURCES: AxolotlKit: :git: https://github.com/WhisperSystems/SignalProtocolKit.git SignalServiceKit: - :branch: mkirk/webrtc + :branch: charlesmchen/webrtc/threadSafety :git: https://github.com/WhisperSystems/SignalServiceKit.git SocketRocket: :git: https://github.com/facebook/SocketRocket.git @@ -141,7 +141,7 @@ CHECKOUT OPTIONS: :commit: 919d541d6b8a8802a94f943026b8f68394e2c0b8 :git: https://github.com/WhisperSystems/SignalProtocolKit.git SignalServiceKit: - :commit: 6521a80c44d3db20a0bd5acc6af2e30a9dbcbd6b + :commit: ddbc4819f1aa10e0164c1591dfc12cba49e04ed0 :git: https://github.com/WhisperSystems/SignalServiceKit.git SocketRocket: :commit: 41b57bb2fc292a814f758441a05243eb38457027 @@ -173,6 +173,6 @@ SPEC CHECKSUMS: YapDatabase: b1e43555a34a5298e23a045be96817a5ef0da58f ZXingObjC: bf15b3814f7a105b6d99f47da2333c93a063650a -PODFILE CHECKSUM: 9f2e2cf6d4db276e3d0280343260503d09c72979 +PODFILE CHECKSUM: beb37f2fe7d11b6d2a8124b03b20f903908fd8aa COCOAPODS: 1.0.1 diff --git a/Signal/src/Signal-Bridging-Header.h b/Signal/src/Signal-Bridging-Header.h index 357ee43e4..0e314e27b 100644 --- a/Signal/src/Signal-Bridging-Header.h +++ b/Signal/src/Signal-Bridging-Header.h @@ -13,6 +13,7 @@ #import "OWSContactAvatarBuilder.h" #import "OWSContactsManager.h" #import "OWSDispatch.h" +#import "OWSError.h" #import "OWSLogger.h" #import "OWSWebRTCDataProtos.pb.h" #import "PhoneNumber.h" diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index 32b9a1663..2345f8a9a 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -635,10 +635,10 @@ protocol CallServiceObserver: class { callRecord.save() let message = DataChannelMessage.forConnected(callId: call.signalingId) - if peerConnectionClient.sendDataChannelMessage(data: message.asData()) { - Logger.debug("\(TAG) sendDataChannelMessage returned true") - } else { - Logger.warn("\(TAG) sendDataChannelMessage returned false") + peerConnectionClient.sendDataChannelMessage(data: message.asData()).then { _ in + Logger.debug("\(self.TAG) sendDataChannelMessage succeeded") + }.catch(on: DispatchQueue.main) { _ in + Logger.warn("\(self.TAG) sendDataChannelMessage failed") } handleConnectedCall(call) @@ -737,10 +737,10 @@ protocol CallServiceObserver: class { // If the call is connected, we can send the hangup via the data channel. let message = DataChannelMessage.forHangup(callId: call.signalingId) - if peerConnectionClient.sendDataChannelMessage(data: message.asData()) { - Logger.debug("\(TAG) sendDataChannelMessage returned true") - } else { - Logger.warn("\(TAG) sendDataChannelMessage returned false") + peerConnectionClient.sendDataChannelMessage(data: message.asData()).then { _ in + Logger.debug("\(self.TAG) sendDataChannelMessage succeeded") + }.catch(on: DispatchQueue.main) { _ in + Logger.warn("\(self.TAG) sendDataChannelMessage failed") } // If the call hasn't started yet, we don't have a data channel to communicate the hang up. Use Signal Service Message. @@ -1075,10 +1075,10 @@ protocol CallServiceObserver: class { self.peerConnectionClient?.setLocalVideoEnabled(enabled: shouldHaveLocalVideoTrack) let message = DataChannelMessage.forVideoStreamingStatus(callId: call.signalingId, enabled:shouldHaveLocalVideoTrack) - if peerConnectionClient.sendDataChannelMessage(data: message.asData()) { - Logger.debug("\(self.TAG) sendDataChannelMessage returned true") - } else { - Logger.warn("\(self.TAG) sendDataChannelMessage returned false") + peerConnectionClient.sendDataChannelMessage(data: message.asData()).then { _ in + Logger.debug("\(self.TAG) sendDataChannelMessage succeeded") + }.catch(on: DispatchQueue.main) { _ in + Logger.warn("\(self.TAG) sendDataChannelMessage failed") } } diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift index daa8307ba..e5852806e 100644 --- a/Signal/src/call/PeerConnectionClient.swift +++ b/Signal/src/call/PeerConnectionClient.swift @@ -80,7 +80,7 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD private weak var delegate: PeerConnectionClientDelegate! func setDelegate(delegate: PeerConnectionClientDelegate?) { - PeerConnectionClient.signalingQueue.sync { + PeerConnectionClient.signalingQueue.async { self.delegate = delegate } } @@ -112,6 +112,8 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD private var cameraConstraints: RTCMediaConstraints init(iceServers: [RTCIceServer], delegate: PeerConnectionClientDelegate) { + AssertIsOnMainThread() + self.iceServers = iceServers self.delegate = delegate @@ -140,6 +142,8 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD // MARK: - Media Streams public func createSignalingDataChannel() { + AssertIsOnMainThread() + PeerConnectionClient.signalingQueue.sync { let dataChannel = peerConnection.dataChannel(forLabel: Identifiers.dataChannelSignaling.rawValue, configuration: RTCDataChannelConfiguration()) @@ -400,23 +404,26 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD // MARK: - Data Channel - public func sendDataChannelMessage(data: Data) -> Bool { + public func sendDataChannelMessage(data: Data) -> Promise { AssertIsOnMainThread() + + return Promise { fulfill, reject in + AssertIsOnMainThread() + + PeerConnectionClient.signalingQueue.async { + self.assertOnSignalingQueue() - var result = false - PeerConnectionClient.signalingQueue.sync { - assertOnSignalingQueue() - - guard let dataChannel = self.dataChannel else { - Logger.error("\(self.TAG) in \(#function) ignoring sending \(data) for nil dataChannel") - result = false - return + guard let dataChannel = self.dataChannel else { + Logger.error("\(self.TAG) in \(#function) ignoring sending \(data) for nil dataChannel") + reject(OWSErrorMakeWebRTCMissingDataChannelError()) + return + } + + let buffer = RTCDataBuffer(data: data, isBinary: false) + let result = dataChannel.sendData(buffer) + fulfill(result) } - - let buffer = RTCDataBuffer(data: data, isBinary: false) - result = dataChannel.sendData(buffer) } - return result } // MARK: RTCDataChannelDelegate diff --git a/Signal/translations/en.lproj/Localizable.strings b/Signal/translations/en.lproj/Localizable.strings index f5bd472fc..7618241da 100644 --- a/Signal/translations/en.lproj/Localizable.strings +++ b/Signal/translations/en.lproj/Localizable.strings @@ -250,6 +250,9 @@ /* Error message when attempting to send message */ "ERROR_DESCRIPTION_UNREGISTERED_RECIPIENT" = "Recipient is no longer a Signal user."; +/* Missing data channel while trying to sending message */ +"ERROR_DESCRIPTION_WEBRTC_MISSING_DATA_CHANNEL" = "Missing connection"; + /* No comment provided by engineer. */ "ERROR_MESSAGE_DUPLICATE_MESSAGE" = "Received a duplicated message.";