From 4f7e6f94647d242f6c86b1512364ead2a210e945 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Mon, 26 Nov 2018 12:52:55 -0800 Subject: [PATCH] client: fix races in use of goroutine group The group utility struct does not support asynchronously launched goroutines (goroutines-inside-of-goroutines), so switch those uses to a normal go call. This means watchNodeUpdates and watchNodeEvents may not be shutdown when Shutdown() exits. During nomad agent shutdown this does not matter. During tests this means a test may leak those goroutines or be unable to know when those goroutines have exited. Since there's no runtime impact and these goroutines do not affect alloc state syncing it seems ok to risk leaking them. --- client/client.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/client/client.go b/client/client.go index 6657b6e8d..6714b0394 100644 --- a/client/client.go +++ b/client/client.go @@ -1215,10 +1215,10 @@ func (c *Client) registerAndHeartbeat() { c.retryRegisterNode() // Start watching changes for node changes - c.shutdownGroup.Go(c.watchNodeUpdates) + go c.watchNodeUpdates() // Start watching for emitting node events - c.shutdownGroup.Go(c.watchNodeEvents) + go c.watchNodeEvents() // Setup the heartbeat timer, for the initial registration // we want to do this quickly. We want to do it extra quickly @@ -2600,6 +2600,7 @@ type group struct { wg sync.WaitGroup } +// Go starts f in a goroutine and must be called before Wait. func (g *group) Go(f func()) { g.wg.Add(1) go func() { @@ -2608,6 +2609,8 @@ func (g *group) Go(f func()) { }() } +// Wait for all goroutines to exit. Must be called after all calls to Go +// complete. func (g *group) Wait() { g.wg.Wait() }