2 files changed, 63 insertions, 17 deletions

diff --git a/p2p/nat/nat.go b/p2p/nat/nat.go
index fe00bdab0..58d50976d 100644
--- a/p2p/nat/nat.go
+++ b/p2p/nat/nat.go
@@ -172,8 +172,9 @@ func PMP(gateway net.IP) Interface {
 // This type is useful because discovery can take a while but we
 // want return an Interface value from UPnP, PMP and Auto immediately.
 type autodisc struct {
-	what string
-	done <-chan Interface
+	what string // type of interface being autodiscovered
+	once sync.Once
+	doit func() Interface
 
 	mu    sync.Mutex
 	found Interface
@@ -181,9 +182,10 @@ type autodisc struct {
 
 func startautodisc(what string, doit func() Interface) Interface {
 	// TODO: monitor network configuration and rerun doit when it changes.
-	done := make(chan Interface)
-	ad := &autodisc{what: what, done: done}
-	go func() { done <- doit(); close(done) }()
+	ad := &autodisc{what: what, doit: doit}
+	// Start the auto discovery as early as possible so it is already
+	// in progress when the rest of the stack calls the methods.
+	go ad.wait()
 	return ad
 }
 
@@ -218,19 +220,15 @@ func (n *autodisc) String() string {
 	}
 }
 
+// wait blocks until auto-discovery has been performed.
 func (n *autodisc) wait() error {
-	n.mu.Lock()
-	found := n.found
-	n.mu.Unlock()
-	if found != nil {
-		// already discovered
-		return nil
-	}
-	if found = <-n.done; found == nil {
-		return errors.New("no UPnP or NAT-PMP router discovered")
+	n.once.Do(func() {
+		n.mu.Lock()
+		n.found = n.doit()
+		n.mu.Unlock()
+	})
+	if n.found == nil {
+		return fmt.Errorf("no %s router discovered", n.what)
 	}
-	n.mu.Lock()
-	n.found = found
-	n.mu.Unlock()
 	return nil
 }
diff --git a/p2p/nat/nat_test.go b/p2p/nat/nat_test.go
new file mode 100644
index 000000000..95c50522e
--- /dev/null
+++ b/p2p/nat/nat_test.go
@@ -0,0 +1,48 @@
+package nat
+
+import (
+	"bytes"
+	"net"
+	"testing"
+	"time"
+)
+
+// This test checks that autodisc doesn't hang and returns
+// consistent results when multiple goroutines call its methods
+// concurrently.
+func TestAutoDiscRace(t *testing.T) {
+	ad := startautodisc("thing", func() Interface {
+		time.Sleep(500 * time.Millisecond)
+		return extIP{33, 44, 55, 66}
+	})
+
+	// Spawn a few concurrent calls to ad.ExternalIP.
+	type rval struct {
+		ip  net.IP
+		err error
+	}
+	results := make(chan rval, 50)
+	for i := 0; i < cap(results); i++ {
+		go func() {
+			ip, err := ad.ExternalIP()
+			results <- rval{ip, err}
+		}()
+	}
+
+	// Check that they all return the correct result within the deadline.
+	deadline := time.After(550 * time.Millisecond)
+	for i := 0; i < cap(results); i++ {
+		select {
+		case <-deadline:
+			t.Fatal("deadline exceeded")
+		case rval := <-results:
+			if rval.err != nil {
+				t.Errorf("result %d: unexpected error: %v", i, rval.err)
+			}
+			wantIP := net.IP{33, 44, 55, 66}
+			if !bytes.Equal(rval.ip, wantIP) {
+				t.Errorf("result %d: got IP %v, want %v", i, rval.ip, wantIP)
+			}
+		}
+	}
+}