nsqd: fix lack of synchronization around ChannelStats

[daroot]

• create inFlightMessages and deferredMessages maps inside mutex.
• acquire appropriate mutexes before doing len() in NewChannelStats()
• add test that checks for passing -race when accessing stats while
  serving messages.

Fixes issue #970.

[ploxiln]

Ah, I see - we can't "atomically" get the len() of a map. This change looks good to me.

[mreiferson]

Let's tighten up the critical sections between these locks and move the len(...) reads from out of the struct instantiation to temp variables here?

[ploxiln]

Awesome. Can you squash down to one commit?

[ploxiln]

Your test did seem to expose a remaining data race in go-1.7:

WARNING: DATA RACE
Write at 0x00c4201b4630 by goroutine 178:
  runtime.mapassign1()
      /home/travis/.gimme/versions/go1.7.6.linux.amd64/src/runtime/hashmap.go:442 +0x0
  github.com/nsqio/nsq/nsqd.(*Channel).pushInFlightMessage()
      /home/travis/gopath/src/github.com/nsqio/nsq/nsqd/channel.go:451 +0x184
  github.com/nsqio/nsq/nsqd.(*Channel).StartInFlightTimeout()
      /home/travis/gopath/src/github.com/nsqio/nsq/nsqd/channel.go:424 +0x14e
  github.com/nsqio/nsq/nsqd.TestStatsChannelLocking.func1()
      /home/travis/gopath/src/github.com/nsqio/nsq/nsqd/stats_test.go:130 +0x109
Previous read at 0x00c4201b4630 by goroutine 38:
  github.com/nsqio/nsq/nsqd.NewChannelStats()
      /home/travis/gopath/src/github.com/nsqio/nsq/nsqd/stats.go:62 +0x176
  github.com/nsqio/nsq/nsqd.(*NSQD).GetStats()
      /home/travis/gopath/src/github.com/nsqio/nsq/nsqd/stats.go:164 +0xc2b
  github.com/nsqio/nsq/nsqd.TestStatsChannelLocking.func2()
      /home/travis/gopath/src/github.com/nsqio/nsq/nsqd/stats_test.go:137 +0x6d

Is it just in the test itself possibly? I may have time to take a look later.

(I manually triggered a re-test and it passed :P)

[ploxiln]

Ah, OK, obviously you need to call len() while locking the mutex.

[daroot]

My bad. I didn't even think about the fact that copying the map is actually a reference to the map. So using a local copy puts you right back to where you started, with the actual map object being accessed from outside the lock. Fix up in a few minutes.

[daroot]

Okay, fix is up, and commits squashed.

[mreiferson]

thanks!