From 62c1bc0a207bdd897eaefefe2d17ec78e5083ef1 Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Mon, 13 Nov 2017 16:28:14 -0700 Subject: [PATCH] p2p: comment on the wg.Add before go saveRoutine() Just noticed while auditing the code in p2p/addrbook.go, wg.Add(1) but no subsequent defer. @jaekwon and I had a discussion offline and we agreed to comment about why the code was that way and why we shouldn't move the wg.Add(1) into .saveRoutine() because if go a.saveRoutine() isn't started before anyone invokes a.Wait(), then we'd have raced a.saveRoutine(). --- p2p/addrbook.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/p2p/addrbook.go b/p2p/addrbook.go index 62b25a710..06162e2e6 100644 --- a/p2p/addrbook.go +++ b/p2p/addrbook.go @@ -127,8 +127,12 @@ func (a *AddrBook) init() { func (a *AddrBook) OnStart() error { a.BaseService.OnStart() a.loadFromFile(a.filePath) + + // wg.Add to ensure that any invocation of .Wait() + // later on will wait for saveRoutine to terminate. a.wg.Add(1) go a.saveRoutine() + return nil } @@ -391,6 +395,8 @@ func (a *AddrBook) Save() { /* Private methods */ func (a *AddrBook) saveRoutine() { + defer a.wg.Done() + dumpAddressTicker := time.NewTicker(dumpAddressInterval) out: for { @@ -403,7 +409,6 @@ out: } dumpAddressTicker.Stop() a.saveToFile(a.filePath) - a.wg.Done() a.Logger.Info("Address handler done") }