aboutsummaryrefslogtreecommitdiffstats
path: root/p2p
diff options
context:
space:
mode:
authorFelix Lange <fjl@twurst.com>2015-05-13 22:04:06 +0800
committerFelix Lange <fjl@twurst.com>2015-05-14 09:53:11 +0800
commit983f5a717a35a604847ae4a4964c19000dc44016 (patch)
treed8f667ef3ab140c3f8809ba41722e8a320d75e34 /p2p
parentf7fdb4dfbe1a0851b6fb2970b0a6b11fd31273d6 (diff)
downloaddexon-983f5a717a35a604847ae4a4964c19000dc44016.tar.gz
dexon-983f5a717a35a604847ae4a4964c19000dc44016.tar.zst
dexon-983f5a717a35a604847ae4a4964c19000dc44016.zip
p2p/nat: fix concurrent access to autodisc Interface
Concurrent calls to Interface methods on autodisc could return a "not discovered" error if the discovery did not finish before the call. autodisc.wait expected the done channel to carry the found Interface but it was closed instead. The fix is to use sync.Once for now, which is easier to get right. And there is a test. Finally. This will have to change again when we introduce re-discovery.
Diffstat (limited to 'p2p')
-rw-r--r--p2p/nat/nat.go32
-rw-r--r--p2p/nat/nat_test.go48
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)
+ }
+ }
+ }
+}