From 325901c2c3e6aa80ff83ebdfa73f5046dbd1b596 Mon Sep 17 00:00:00 2001
From: Zach Klippenstein <zach.klippenstein@gmail.com>
Date: Wed, 30 Dec 2015 00:00:03 -0800
Subject: [PATCH] Fixed bug where tried closing a single net.Conn multiple
 times.

---
 dialer.go           | 18 ++++++++++++------
 sync_file_writer.go |  2 +-
 wire/util.go        | 20 ++++++++++++++++++++
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/dialer.go b/dialer.go
index ebb5c12..83334a6 100644
--- a/dialer.go
+++ b/dialer.go
@@ -2,6 +2,7 @@ package goadb
 
 import (
 	"fmt"
+	"io"
 	"net"
 	"runtime"
 
@@ -66,17 +67,22 @@ func (d *netDialer) Dial() (*wire.Conn, error) {
 		}
 	}
 
-	conn := &wire.Conn{
-		Scanner: wire.NewScanner(netConn),
-		Sender:  wire.NewSender(netConn),
-	}
+	// net.Conn can't be closed more than once, but wire.Conn will try to close both sender and scanner
+	// so we need to wrap it to make it safe.
+	safeConn := wire.MultiCloseable(netConn)
 
 	// Prevent leaking the network connection, not sure if TCPConn does this itself.
-	runtime.SetFinalizer(netConn, func(conn *net.TCPConn) {
+	// Note that the network connection may still be in use after the conn isn't (scanners/senders
+	// can give their underlying connections to other scanner/sender types), so we can't
+	// set the finalizer on conn.
+	runtime.SetFinalizer(safeConn, func(conn io.ReadWriteCloser) {
 		conn.Close()
 	})
 
-	return conn, nil
+	return &wire.Conn{
+		Scanner: wire.NewScanner(safeConn),
+		Sender:  wire.NewSender(safeConn),
+	}, nil
 }
 
 func roundTripSingleResponse(d Dialer, req string) ([]byte, error) {
diff --git a/sync_file_writer.go b/sync_file_writer.go
index 35130df..d816d4e 100644
--- a/sync_file_writer.go
+++ b/sync_file_writer.go
@@ -66,7 +66,7 @@ func (w *syncFileWriter) Close() error {
 	}
 
 	if err := w.sender.SendOctetString(wire.StatusSyncDone); err != nil {
-		return util.WrapErrf(err, "error closing file writer")
+		return util.WrapErrf(err, "error sending done chunk to close stream")
 	}
 	if err := w.sender.SendTime(w.mtime); err != nil {
 		return util.WrapErrf(err, "error writing file modification time")
diff --git a/wire/util.go b/wire/util.go
index 0a0c5e1..63bb7fc 100644
--- a/wire/util.go
+++ b/wire/util.go
@@ -5,6 +5,8 @@ import (
 	"io"
 	"regexp"
 
+	"sync"
+
 	"github.com/zach-klippenstein/goadb/util"
 )
 
@@ -80,3 +82,21 @@ func writeFully(w io.Writer, data []byte) error {
 	}
 	return nil
 }
+
+// MultiCloseable wraps c in a ReadWriteCloser that can be safely closed multiple times.
+func MultiCloseable(c io.ReadWriteCloser) io.ReadWriteCloser {
+	return &multiCloseable{ReadWriteCloser: c}
+}
+
+type multiCloseable struct {
+	io.ReadWriteCloser
+	closeOnce sync.Once
+	err       error
+}
+
+func (c *multiCloseable) Close() error {
+	c.closeOnce.Do(func() {
+		c.err = c.ReadWriteCloser.Close()
+	})
+	return c.err
+}