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 + 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) } - if found = <-n.done; found == nil { - return errors.New("no UPnP or NAT-PMP router discovered") - } - 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) + } + } + } +}