[tor-commits] [snowflake/master] defer conn.Close for simplicity and remove unnecessary goroutines, improve error handling (close #12)

[arlo at torproject.org]

commit b8815627bd189c3ee0829ce09152684726c6933c
Author: Serene Han <keroserene+git at gmail.com>
Date:   Sat Mar 19 22:33:54 2016 -0700

    defer conn.Close for simplicity and remove unnecessary goroutines, improve error handling (close #12)
---
 client/snowflake.go | 30 ++++++++++--------------------
 client/webrtc.go    | 22 ++++++++++------------
 2 files changed, 20 insertions(+), 32 deletions(-)

diff --git a/client/snowflake.go b/client/snowflake.go
index 5fe55a9..15f9b37 100644
--- a/client/snowflake.go
+++ b/client/snowflake.go
@@ -35,15 +35,12 @@ const (
 func copyLoop(a, b net.Conn) {
 	var wg sync.WaitGroup
 	wg.Add(2)
-	// TODO fix copy loop recovery
 	go func() {
-		written, err := io.Copy(b, a)
-		log.Println("copy loop b-a break", err, written)
+		io.Copy(b, a)
 		wg.Done()
 	}()
 	go func() {
-		written, err := io.Copy(a, b)
-		log.Println("copy loop a-b break", err, written)
+		io.Copy(a, b)
 		wg.Done()
 	}()
 	wg.Wait()
@@ -66,11 +63,9 @@ func dialWebRTC() (*webRTCConn, error) {
 	if nil == broker {
 		return nil, errors.New("Failed to prepare BrokerChannel")
 	}
-
 	connection := NewWebRTCConnection(config, broker)
-	go connection.ConnectLoop()
-
-	return connection, nil
+	err := connection.Connect()
+	return connection, err
 }
 
 func endWebRTC() {
@@ -97,14 +92,9 @@ func handler(conn *pt.SocksConn) error {
 		return err
 	}
 	defer remote.Close()
+	defer conn.Close()
 	webrtcRemote = remote
 
-	// Induce another call to handler
-	go func() {
-		<-remote.reset
-		conn.Close()
-	}()
-
 	err = conn.Grant(&net.TCPAddr{IP: net.IPv4zero, Port: 0})
 	if err != nil {
 		return err
@@ -112,8 +102,10 @@ func handler(conn *pt.SocksConn) error {
 
 	// TODO: Make SOCKS acceptance more independent from WebRTC so they can
 	// be more easily interchanged.
-	copyLoop(conn, remote)
-	log.Println("----END---")
+	go copyLoop(conn, remote)
+	// When WebRTC resets, close the SOCKS connection, which induces new handler.
+	<-remote.reset
+	log.Println("---- Closed ---")
 	return nil
 }
 
@@ -159,7 +151,6 @@ func readSignalingMessages(f *os.File) {
 }
 
 func main() {
-	// var err error
 	webrtc.SetLoggingVerbosity(1)
 	logFile, err := os.OpenFile("snowflake.log", os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600)
 	if err != nil {
@@ -174,8 +165,7 @@ func main() {
 	flag.Var(&iceServers, "ice", "comma-separated list of ICE servers")
 	flag.Parse()
 
-	// Expect user to copy-paste if
-	// TODO: Maybe just get rid of copy-paste entirely.
+	// TODO: Maybe just get rid of copy-paste option entirely.
 	if "" != brokerURL {
 		log.Println("Rendezvous using Broker at: ", brokerURL)
 		if "" != frontDomain {
diff --git a/client/webrtc.go b/client/webrtc.go
index 2be31b5..1300212 100644
--- a/client/webrtc.go
+++ b/client/webrtc.go
@@ -2,6 +2,7 @@ package main
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"github.com/keroserene/go-webrtc"
 	"io"
@@ -99,26 +100,24 @@ func NewWebRTCConnection(config *webrtc.Configuration,
 	return connection
 }
 
-// WebRTC re-establishment loop. Expected in own goroutine.
-func (c *webRTCConn) ConnectLoop() {
+// TODO: Multiplex.
+func (c *webRTCConn) Connect() error {
 	log.Println("Establishing WebRTC connection...")
 	// TODO: When go-webrtc is more stable, it's possible that a new
 	// PeerConnection won't need to be re-prepared each time.
 	err := c.preparePeerConnection()
 	if err != nil {
-		log.Println("WebRTC: Could not create PeerConnection.")
-		return
+		return err
 	}
 	err = c.establishDataChannel()
 	if err != nil {
-		log.Println("WebRTC: Could not establish DataChannel.")
-	} else {
-		go c.exchangeSDP()
-		<-c.reset
-		log.Println(" --- snowflake connection reset ---")
+		return errors.New("WebRTC: Could not establish DataChannel.")
 	}
-	<-time.After(time.Second * 1)
-	c.cleanup()
+	err = c.exchangeSDP()
+	if err != nil {
+		return err
+	}
+	return nil
 }
 
 // Create and prepare callbacks on a new WebRTC PeerConnection.
@@ -277,7 +276,6 @@ func (c *webRTCConn) exchangeSDP() error {
 			answer = nil
 		}
 	}
-
 	log.Printf("Received Answer:\n\n%s\n", answer.Sdp)
 	err := c.pc.SetRemoteDescription(answer)
 	if nil != err {