[tor-commits] [snowflake/master] Fix for proxy deadlock bug

[cohosh at torproject.org]

commit c28c8ca489633aae2d9b9dbea0e781ca5e44cc66
Author: Cecylia Bocovich <cohosh at torproject.org>
Date:   Sat Mar 30 12:19:29 2019 -0400

    Fix for proxy deadlock bug
    
    This is a fix for the proxy-go deadlock bug (ticket #25688). The
    assumption that OnIceComplete is always followed by a successful
    connection where OnDataChannel has been called turns out not to occur in
    practice. OnICEComplete looks like it is being deprecated in other
    libraries anyway, so it's safer to just remove it.
---
 proxy-go/snowflake.go | 42 ++++++++++++++++--------------------------
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/proxy-go/snowflake.go b/proxy-go/snowflake.go
index 2e8fe20..aeb9f51 100644
--- a/proxy-go/snowflake.go
+++ b/proxy-go/snowflake.go
@@ -259,8 +259,6 @@ func datachannelHandler(conn *webRTCConn, remoteAddr net.Addr) {
 // Installs an OnDataChannel callback that creates a webRTCConn and passes it to
 // datachannelHandler.
 func makePeerConnectionFromOffer(sdp *webrtc.SessionDescription, config *webrtc.Configuration) (*webrtc.PeerConnection, error) {
-	errChan := make(chan error)
-	answerChan := make(chan struct{})
 
 	pc, err := webrtc.NewPeerConnection(config)
 	if err != nil {
@@ -269,9 +267,6 @@ func makePeerConnectionFromOffer(sdp *webrtc.SessionDescription, config *webrtc.
 	pc.OnNegotiationNeeded = func() {
 		panic("OnNegotiationNeeded")
 	}
-	pc.OnIceComplete = func() {
-		answerChan <- struct{}{}
-	}
 	pc.OnDataChannel = func(dc *webrtc.DataChannel) {
 		log.Println("OnDataChannel")
 
@@ -310,30 +305,25 @@ func makePeerConnectionFromOffer(sdp *webrtc.SessionDescription, config *webrtc.
 	}
 	log.Println("sdp offer successfully received.")
 
-	go func() {
-		log.Println("Generating answer...")
-		answer, err := pc.CreateAnswer() // blocking
-		if err != nil {
-			errChan <- err
-			return
-		}
-		err = pc.SetLocalDescription(answer)
-		if err != nil {
-			errChan <- err
-			return
-		}
-	}()
+	log.Println("Generating answer...")
+	answer, err := pc.CreateAnswer()
+        // blocks on ICE gathering. we need to add a timeout if needed
+        // not putting this in a separate go routine, because we need
+        // SetLocalDescription(answer) to be called before sendAnswer
+	if err != nil {
+		pc.Destroy()
+		return nil, err
+	}
 
-	// Wait until answer is ready.
-	select {
-	case err = <-errChan:
+	if answer == nil {
+		pc.Destroy()
+		return nil, fmt.Errorf("Failed gathering ICE candidates.")
+	}
+
+	err = pc.SetLocalDescription(answer)
+	if err != nil {
 		pc.Destroy()
 		return nil, err
-	case _, ok := <-answerChan:
-		if !ok {
-			pc.Destroy()
-			return nil, fmt.Errorf("Failed gathering ICE candidates.")
-		}
 	}
 	return pc, nil
 }