[tor-commits] [snowflake/master] defer snowflake.Reset to fix handler recovery when localhost SOCKS occassionally fails first

[serene at torproject.org]

commit ea2e052a7dedbba4ae3cc2216acd98f86b788d99
Author: Serene H <git at keroserene.net>
Date:   Fri Jul 29 10:59:51 2016 -0700

    defer snowflake.Reset to fix handler recovery when localhost SOCKS occassionally fails first
---
 client/client_test.go |  4 ++--
 client/rendezvous.go  |  6 +++---
 client/snowflake.go   |  7 ++++---
 client/torrc          |  2 +-
 client/webrtc.go      | 12 +++++++-----
 5 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/client/client_test.go b/client/client_test.go
index 20614fa..cfc8cbf 100644
--- a/client/client_test.go
+++ b/client/client_test.go
@@ -184,11 +184,11 @@ func TestSnowflakeClient(t *testing.T) {
 		})
 
 		Convey("WebRTC Connection", func() {
-			c := NewWebRTCConnection(nil, nil)
+			c := NewWebRTCPeer(nil, nil)
 			So(c.buffer.Bytes(), ShouldEqual, nil)
 
 			Convey("Can construct a WebRTCConn", func() {
-				s := NewWebRTCConnection(nil, nil)
+				s := NewWebRTCPeer(nil, nil)
 				So(s, ShouldNotBeNil)
 				So(s.offerChannel, ShouldNotBeNil)
 				So(s.answerChannel, ShouldNotBeNil)
diff --git a/client/rendezvous.go b/client/rendezvous.go
index 6933df9..0aed3f7 100644
--- a/client/rendezvous.go
+++ b/client/rendezvous.go
@@ -138,7 +138,7 @@ func (w WebRTCDialer) Catch() (Snowflake, error) {
 	}
 	// TODO: [#3] Fetch ICE server information from Broker.
 	// TODO: [#18] Consider TURN servers here too.
-	connection := NewWebRTCConnection(w.webrtcConfig, w.BrokerChannel)
+	connection := NewWebRTCPeer(w.webrtcConfig, w.BrokerChannel)
 	err := connection.Connect()
 	return connection, err
 }
@@ -177,12 +177,12 @@ func NewCopyPasteDialer(iceServers IceServerList) *CopyPasteDialer {
 	return dialer
 }
 
-// Initialize a WebRTC connection via manual copy-paste.
+// Initialize a WebRTC Peer via manual copy-paste.
 func (d *CopyPasteDialer) Catch() (Snowflake, error) {
 	if nil == d.signal {
 		return nil, errors.New("Cannot copy-paste dial without signal pipe.")
 	}
-	connection := NewWebRTCConnection(d.webrtcConfig, nil)
+	connection := NewWebRTCPeer(d.webrtcConfig, nil)
 	// Must keep track of pending new connection until copy-paste completes.
 	d.current = connection
 	// Outputs SDP offer to log, expecting user to copy-paste to the remote Peer.
diff --git a/client/snowflake.go b/client/snowflake.go
index 72c3295..8dfa390 100644
--- a/client/snowflake.go
+++ b/client/snowflake.go
@@ -81,6 +81,7 @@ func handler(socks SocksConnector, snowflakes SnowflakeCollector) error {
 		return errors.New("handler: Received invalid Snowflake")
 	}
 	defer socks.Close()
+	defer snowflake.Reset()
 	log.Println("---- Handler: snowflake assigned ----")
 	err := socks.Grant(&net.TCPAddr{IP: net.IPv4zero, Port: 0})
 	if err != nil {
@@ -88,13 +89,13 @@ func handler(socks SocksConnector, snowflakes SnowflakeCollector) error {
 	}
 
 	go func() {
-		// When WebRTC resets, close the SOCKS connection, which ends
-		// the copyLoop below and induces new handler.
+		// When WebRTC resets, close the SOCKS connection too.
 		snowflake.WaitForReset()
 		socks.Close()
 	}()
 
-	// Begin exchanging data.
+	// Begin exchanging data. Either WebRTC or localhost SOCKS will close first.
+	// In eithercase, this closes the handler and induces a new handler.
 	copyLoop(socks, snowflake)
 	log.Println("---- Handler: closed ---")
 	return nil
diff --git a/client/torrc b/client/torrc
index 6912760..d066454 100644
--- a/client/torrc
+++ b/client/torrc
@@ -5,6 +5,6 @@ ClientTransportPlugin snowflake exec ./client \
 -url https://snowflake-reg.appspot.com/ \
 -front www.google.com \
 -ice stun:stun.l.google.com:19302 \
--max 3
+-max 1
 
 Bridge snowflake 0.0.3.0:1
diff --git a/client/webrtc.go b/client/webrtc.go
index 62a59a0..1f7ac00 100644
--- a/client/webrtc.go
+++ b/client/webrtc.go
@@ -58,12 +58,14 @@ func (c *WebRTCPeer) Write(b []byte) (int, error) {
 
 // As part of |Snowflake|
 func (c *WebRTCPeer) Close() error {
-	var err error = nil
+	if c.closed { // Skip if already closed.
+		return nil
+	}
 	log.Printf("WebRTC: Closing")
 	c.cleanup()
 	// Mark for deletion.
 	c.closed = true
-	return err
+	return nil
 }
 
 // As part of |Resetter|
@@ -79,7 +81,7 @@ func (c *WebRTCPeer) Reset() {
 func (c *WebRTCPeer) WaitForReset() { <-c.reset }
 
 // Construct a WebRTC PeerConnection.
-func NewWebRTCConnection(config *webrtc.Configuration,
+func NewWebRTCPeer(config *webrtc.Configuration,
 	broker *BrokerChannel) *WebRTCPeer {
 	connection := new(WebRTCPeer)
 	connection.id = "snowflake-" + uniuri.New()
@@ -298,8 +300,8 @@ func (c *WebRTCPeer) cleanup() {
 	if nil != c.transport {
 		log.Printf("WebRTC: closing DataChannel")
 		dataChannel := c.transport
-		// Setting dc to nil *before* Close indicates to OnClose that it
-		// was locally triggered.
+		// Setting transport to nil *before* dc Close indicates to OnClose that
+		// this was locally triggered.
 		c.transport = nil
 		dataChannel.Close()
 	}