Repository: gammazero/nexus (go)

Issue:
Thread saftey
Is the API thread safe? I'm getting really strange bugs and think this may be due to data races. Eg
```
WARNING: DATA RACE
Read at 0x00c4200165b0 by goroutine 21:
  github.com/gammazero/nexus/client.(*Client).CallProgress()
      /Users/nils/Work/go/src/github.com/gammazero/nexus/wamp/idgen.go:41 +0xdd
  github.com/gammazero/nexus/client.(*Client).Call()
...
Previous write at 0x00c4200165b0 by goroutine 20:
  github.com/gammazero/nexus/client.(*Client).CallProgress()
      /Users/nils/Work/go/src/github.com/gammazero/nexus/wamp/idgen.go:41 +0xf9
  github.com/gammazero/nexus/client.(*Client).Call()
```

Do I need to serialize these calls?

Pull request:
Fix race in client when making concurrent requests in same session
Fixes #84 

Patch:
diff --git a/client/client.go b/client/client.go
index b3031a5..9e36e9f 100644
--- a/client/client.go
+++ b/client/client.go
@@ -138,7 +138,7 @@ type Client struct {
 	progGate       map[context.Context]wamp.ID
 
 	actionChan chan func()
-	idGen      *wamp.IDGen
+	idGen      *wamp.SyncIDGen
 
 	stopping          chan struct{}
 	activeInvHandlers sync.WaitGroup
@@ -186,7 +186,7 @@ func NewClient(p wamp.Peer, cfg ClientConfig) (*Client, error) {
 		progGate:       map[context.Context]wamp.ID{},
 
 		actionChan: make(chan func()),
-		idGen:      wamp.NewIDGen(),
+		idGen:      new(wamp.SyncIDGen),
 		stopping:   make(chan struct{}),
 		done:       make(chan struct{}),
 
diff --git a/router/broker.go b/router/broker.go
index 770a967..846b85f 100644
--- a/router/broker.go
+++ b/router/broker.go
@@ -78,7 +78,7 @@ func NewBroker(logger stdlog.StdLog, strictURI, allowDisclose, debug bool) *Brok
 		// channel is appropriate.
 		actionChan: make(chan func()),
 
-		idGen: wamp.NewIDGen(),
+		idGen: new(wamp.IDGen),
 
 		strictURI:     strictURI,
 		allowDisclose: allowDisclose,
diff --git a/router/dealer.go b/router/dealer.go
index f534e5d..3811f0e 100644
--- a/router/dealer.go
+++ b/router/dealer.go
@@ -126,7 +126,7 @@ func NewDealer(logger stdlog.StdLog, strictURI, allowDisclose, debug bool) *Deal
 		// channel is appropriate.
 		actionChan: make(chan func()),
 
-		idGen: wamp.NewIDGen(),
+		idGen: new(wamp.IDGen),
 		prng:  rand.New(rand.NewSource(time.Now().Unix())),
 
 		strictURI:     strictURI,
diff --git a/router/realm.go b/router/realm.go
index 953e921..f5b1df6 100644
--- a/router/realm.go
+++ b/router/realm.go
@@ -84,7 +84,7 @@ func newRealm(config *RealmConfig, broker *Broker, dealer *Dealer, logger stdlog
 		clients:     map[wamp.ID]*wamp.Session{},
 		clientStop:  make(chan struct{}),
 		actionChan:  make(chan func()),
-		metaIDGen:   wamp.NewIDGen(),
+		metaIDGen:   new(wamp.IDGen),
 		metaStop:    make(chan struct{}),
 		metaDone:    make(chan struct{}),
 		metaProcMap: make(map[wamp.ID]func(*wamp.Invocation) wamp.Message, 9),
diff --git a/wamp/idgen.go b/wamp/idgen.go
index 422b54a..b995b2c 100644
--- a/wamp/idgen.go
+++ b/wamp/idgen.go
@@ -2,6 +2,7 @@ package wamp
 
 import (
 	"math/rand"
+	"sync"
 	"time"
 )
 
@@ -16,7 +17,7 @@ func GlobalID() ID {
 	return ID(rand.Int63n(maxID))
 }
 
-// ID generator for WAMP request IDs.
+// ID generator for WAMP request IDs.  Create with new(IDGen).
 //
 // WAMP request IDs are sequential per WAMP session, starting at 1 and wrapping
 // around at 2**53 (both value are inclusive [1, 2**53]).
@@ -31,11 +32,6 @@ type IDGen struct {
 	next int64
 }
 
-// NewIDGen a new sequential ID generator.
-func NewIDGen() *IDGen {
-	return &IDGen{}
-}
-
 // Next returns next ID.
 func (g *IDGen) Next() ID {
 	g.next++
@@ -44,3 +40,16 @@ func (g *IDGen) Next() ID {
 	}
 	return ID(g.next)
 }
+
+// SyncIDGen is a concurrent-safe IDGen.  Create with new(SyncIDGen).
+type SyncIDGen struct {
+	IDGen
+	lock sync.Mutex
+}
+
+// Next returns next ID.
+func (g *SyncIDGen) Next() ID {
+	g.lock.Lock()
+	defer g.lock.Unlock()
+	return g.IDGen.Next()
+}


Test patch:
diff --git a/wamp/idgen_test.go b/wamp/idgen_test.go
index a24dd15..5b0d910 100644
--- a/wamp/idgen_test.go
+++ b/wamp/idgen_test.go
@@ -1,6 +1,9 @@
 package wamp
 
-import "testing"
+import (
+	"errors"
+	"testing"
+)
 
 func TestIDGen(t *testing.T) {
 	id1 := GlobalID()
@@ -11,7 +14,7 @@ func TestIDGen(t *testing.T) {
 		t.Fatal("Globals should not be equal")
 	}
 
-	idgen := NewIDGen()
+	idgen := new(IDGen)
 	id1 = idgen.Next()
 	if id1 != 1 {
 		t.Fatal("Sequential IDs should start at 1")
@@ -28,3 +31,34 @@ func TestIDGen(t *testing.T) {
 		t.Fatal("Sequential IDs should wrap at 1 << 53")
 	}
 }
+
+func TestSyncIDGen(t *testing.T) {
+	start := make(chan struct{})
+	errs := make(chan error, 4)
+	idgen := new(SyncIDGen)
+	f := func() {
+		var i, prev ID
+		<-start
+		for j := 0; j < 100000; j++ {
+			i = idgen.Next()
+			if i <= prev {
+				errs <- errors.New("Bad sequential ID")
+				return
+			}
+			prev = i
+		}
+		errs <- nil
+	}
+
+	go f()
+	go f()
+	go f()
+	go f()
+	close(start)
+	for g := 0; g < 4; g++ {
+		err := <-errs
+		if err != nil {
+			t.Fatal(err.Error())
+		}
+	}
+}