Skip to content

Commit

Permalink
producer: fix connection race
Browse files Browse the repository at this point in the history
Prior to this change, it is possible for more than one goroutine to
check:

	if atomic.LoadInt32(&w.state) != StateConnected {

...thereafter calling w.connect.  w.connect first grabs a lock on
w.guard, and then asserts that it be the only goroutine attempting a
connection:

	if !atomic.CompareAndSwapInt32(&w.state, StateInit, StateConnected) {
		return ErrNotConnected
	}

This means if there are two parallel actions which attempt to initiate
the connection, one of them will return ErrNotConnected if the first
succesfully connected.
  • Loading branch information
jeddenlea committed Apr 8, 2015
1 parent a76e331 commit 79c45bd
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions producer.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,11 @@ func (w *Producer) connect() error {
return ErrStopped
}

if !atomic.CompareAndSwapInt32(&w.state, StateInit, StateConnected) {
switch state := atomic.LoadInt32(&w.state); state {
case StateInit:
case StateConnected:
return nil
default:
return ErrNotConnected
}

Expand All @@ -258,9 +262,9 @@ func (w *Producer) connect() error {
if err != nil {
w.conn.Close()
w.log(LogLevelError, "(%s) error connecting to nsqd - %s", w.addr, err)
atomic.StoreInt32(&w.state, StateInit)
return err
}
atomic.StoreInt32(&w.state, StateConnected)
w.closeChan = make(chan int)
w.wg.Add(1)
go w.router()
Expand Down

0 comments on commit 79c45bd

Please sign in to comment.