From d9bcd563b19ca2e8b69c7729290d2d2007eaa9e4 Mon Sep 17 00:00:00 2001
From: Matthew Chen <charlesmchen@gmail.com>
Date: Fri, 3 Feb 2017 16:54:34 -0500
Subject: [PATCH] Avoid possible deadlock in PeerConnectionClient.

// FREEBIE
---
 Signal/src/call/CallService.swift             |  8 ++----
 Signal/src/call/PeerConnectionClient.swift    | 27 ++++++++++++-------
 .../test/call/PeerConnectionClientTest.swift  |  3 +--
 3 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift
index 0cf4cb6a5..1ef4cb966 100644
--- a/Signal/src/call/CallService.swift
+++ b/Signal/src/call/CallService.swift
@@ -283,11 +283,7 @@ protocol CallServiceObserver: class {
         return getIceServers().then { iceServers -> Promise<HardenedRTCSessionDescription> in
             Logger.debug("\(self.TAG) got ice servers:\(iceServers)")
 
-            let peerConnectionClient = PeerConnectionClient(iceServers: iceServers, delegate: self)
-
-            // When placing an outgoing call, it's our responsibility to create the DataChannel. Recipient will not have
-            // to do this explicitly.
-            peerConnectionClient.createSignalingDataChannel()
+            let peerConnectionClient = PeerConnectionClient(iceServers: iceServers, delegate: self, callType: .Outgoing)
 
             assert(self.peerConnectionClient == nil, "Unexpected PeerConnectionClient instance")
             Logger.debug("\(self.TAG) setting peerConnectionClient in \(#function)")
@@ -439,7 +435,7 @@ protocol CallServiceObserver: class {
             // 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)
+            self.peerConnectionClient = PeerConnectionClient(iceServers: iceServers, delegate: self, callType: .Incoming)
 
             let offerSessionDescription = RTCSessionDescription(type: .offer, sdp: callerSessionDescription)
             let constraints = RTCMediaConstraints(mandatoryConstraints: nil, optionalConstraints: nil)
diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift
index 1a6d3dcdc..8dc63b664 100644
--- a/Signal/src/call/PeerConnectionClient.swift
+++ b/Signal/src/call/PeerConnectionClient.swift
@@ -64,6 +64,11 @@ protocol PeerConnectionClientDelegate: class {
  */
 class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelDelegate {
 
+    enum CallType {
+        case Incoming
+        case Outgoing
+    }
+
     let TAG = "[PeerConnectionClient]"
     enum Identifiers: String {
         case mediaStream = "ARDAMS",
@@ -113,7 +118,7 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD
     private var remoteVideoTrack: RTCVideoTrack?
     private var cameraConstraints: RTCMediaConstraints
 
-    init(iceServers: [RTCIceServer], delegate: PeerConnectionClientDelegate) {
+    init(iceServers: [RTCIceServer], delegate: PeerConnectionClientDelegate, callType: CallType) {
         AssertIsOnMainThread()
 
         self.iceServers = iceServers
@@ -139,21 +144,25 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD
                                                 delegate: self)
         createAudioSender()
         createVideoSender()
+
+        if callType == .Outgoing {
+            // When placing an outgoing call, it's our responsibility to create the DataChannel. 
+            // Recipient will not have to do this explicitly.
+            createSignalingDataChannel()
+        }
     }
 
     // MARK: - Media Streams
 
-    public func createSignalingDataChannel() {
+    fileprivate func createSignalingDataChannel() {
         AssertIsOnMainThread()
 
-        PeerConnectionClient.signalingQueue.sync {
-            let dataChannel = peerConnection.dataChannel(forLabel: Identifiers.dataChannelSignaling.rawValue,
-                                                         configuration: RTCDataChannelConfiguration())
-            dataChannel.delegate = self
+        let dataChannel = peerConnection.dataChannel(forLabel: Identifiers.dataChannelSignaling.rawValue,
+                                                     configuration: RTCDataChannelConfiguration())
+        dataChannel.delegate = self
 
-            assert(self.dataChannel == nil)
-            self.dataChannel = dataChannel
-        }
+        assert(self.dataChannel == nil)
+        self.dataChannel = dataChannel
     }
 
     // MARK: Video
diff --git a/Signal/test/call/PeerConnectionClientTest.swift b/Signal/test/call/PeerConnectionClientTest.swift
index 85e615e1f..d9887915a 100644
--- a/Signal/test/call/PeerConnectionClientTest.swift
+++ b/Signal/test/call/PeerConnectionClientTest.swift
@@ -55,9 +55,8 @@ class PeerConnectionClientTest: XCTestCase {
 
         let iceServers = [RTCIceServer]()
         clientDelegate = FakePeerConnectionClientDelegate()
-        client = PeerConnectionClient(iceServers: iceServers, delegate: clientDelegate)
+        client = PeerConnectionClient(iceServers: iceServers, delegate: clientDelegate, callType: .Outgoing)
         peerConnection = client.peerConnectionForTests()
-        client.createSignalingDataChannel()
         dataChannel = client.dataChannelForTests()
     }