From 2310dec6319a3994b0b299bb09e3768d8f1893f3 Mon Sep 17 00:00:00 2001
From: Sarah Jamie Lewis <sarah@openprivacy.ca>
Date: Thu, 8 Apr 2021 18:44:45 -0700
Subject: [PATCH] Fix minor "datarace" caused by unecessary assignment after
 close

---
 service.go | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/service.go b/service.go
index f7b5408..2ba5d65 100644
--- a/service.go
+++ b/service.go
@@ -73,6 +73,7 @@ func NewConnection(service Service, id *primitives.Identity, hostname string, ou
 	connection.outbound = outbound
 	connection.MaxLength = 8192
 	connection.service = service
+
 	go connection.app.Init(connection)
 	return connection
 }
@@ -141,6 +142,10 @@ func (c *connection) HasCapability(name Capability) bool {
 func (c *connection) Close() {
 	c.lock.Lock()
 	defer c.lock.Unlock()
+	c.closeInner()
+}
+
+func (c *connection) closeInner() {
 	c.closed = true
 	c.conn.Close()
 }
@@ -148,12 +153,13 @@ func (c *connection) Close() {
 // Expect blocks and reads a single Tapir packet , from the connection.
 func (c *connection) Expect() []byte {
 	buffer := make([]byte, c.MaxLength)
+	// Multiple goroutines may invoke methods on a Conn simultaneously.
+	// As such we don't need to mutex around closed.
 	n, err := io.ReadFull(c.conn, buffer)
 
 	if n != c.MaxLength || err != nil {
 		log.Debugf("[%v -> %v] Wire Error Reading, Read %d bytes, Error: %v", c.hostname, c.identity.Hostname(), n, err)
-		c.conn.Close()
-		c.closed = true
+		c.Close() // use the full close function which acquires a lock for the connection state...
 		return []byte{}
 	}
 	c.lock.Lock()
@@ -166,8 +172,7 @@ func (c *connection) Expect() []byte {
 			copy(buffer, decrypted)
 		} else {
 			log.Errorf("[%v -> %v] Error Decrypting Message On Wire", c.hostname, c.identity.Hostname())
-			c.conn.Close()
-			c.closed = true
+			c.closeInner()
 			return []byte{}
 		}
 	}
@@ -200,8 +205,7 @@ func (c *connection) Send(message []byte) {
 		var nonce [24]byte
 		if _, err := io.ReadFull(rand.Reader, nonce[:]); err != nil {
 			log.Errorf("Could not read sufficient randomness %v. Closing connection", err)
-			c.conn.Close()
-			c.closed = true
+			c.closeInner()
 		}
 		// MaxLength - 40 = MaxLength - 24 nonce bytes and 16 auth tag.
 		encrypted := secretbox.Seal(nonce[:], buffer[0:c.MaxLength-40], &nonce, &c.key)
@@ -210,8 +214,7 @@ func (c *connection) Send(message []byte) {
 	log.Debugf("[%v -> %v] Wire Send %x", c.identity.Hostname(), c.hostname, buffer)
 	_, err := c.conn.Write(buffer)
 	if err != nil {
-		c.conn.Close()
-		c.closed = true
+		c.closeInner()
 	}
 }