From 51e38d75447409e9f1c9f5eb25653105d80a6e3d Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Sun, 29 Jan 2017 17:15:11 -0800 Subject: [PATCH] server: fix goroutine closure bug when broadcasting messages to peers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- server.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server.go b/server.go index 4245ebf69..464e7c703 100644 --- a/server.go +++ b/server.go @@ -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()