server: fix goroutine closure bug when broadcasting messages to peers

This commit fixes a goroutine closure bug introduced by a prior commit.
A prior commit launched a goroutine for each peer to broadcast the
messages in parallel. However, as written this caused the messages to
only be broadcast to a single peer. When launching goroutines in a
for-loop, the “range” variable is actually re-used and re-assigned
within each iteration of the for-loop. As a result, all goroutines
launched will bind onto the _same_ instance of the variable.

We fix this bug in this commit by properly binding the target peer to a
new variable within the closure that launches the goroutine.

Relevant sources:
  *
https://github.com/golang/go/wiki/CommonMistakes#using-goroutines-on-loo
p-iterator-variables
  * https://golang.org/doc/faq#closures_and_goroutines
This commit is contained in:
Olaoluwa Osuntokun 2017-01-29 17:15:11 -08:00
parent f18a21fc54
commit 51e38d7544
No known key found for this signature in database
GPG Key ID: 9CC5B105D03521A2

View File

@ -584,9 +584,9 @@ out:
// requests.
go func() {
s.peersMtx.RLock()
for _, peer := range s.peersByPub {
for _, sPeer := range s.peersByPub {
if ignore != nil &&
peer.addr.IdentityKey.IsEqual(ignore) {
sPeer.addr.IdentityKey.IsEqual(ignore) {
srvrLog.Debugf("Skipping %v in broadcast",
ignore.SerializeCompressed())
@ -594,11 +594,11 @@ out:
continue
}
go func() {
go func(p *peer) {
for _, msg := range bMsg.msgs {
peer.queueMsg(msg, nil)
p.queueMsg(msg, nil)
}
}()
}(sPeer)
}
s.peersMtx.RUnlock()