From 4235b32a5c72154badebabe1f930be0e1f126007 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Wed, 19 Dec 2018 12:26:57 -0800 Subject: [PATCH] test: fix race in eval broker update chan Similar to previous commits the delayed eval update chan was set and access from different goroutines causing a race. Passing the chan on the stack resolves the race. Race output from `go test -race -run 'Server_RPC$'` in nomad/ ``` ================== WARNING: DATA RACE Write at 0x00c000339150 by goroutine 63: github.com/hashicorp/nomad/nomad.(*EvalBroker).flush() /home/schmichael/go/src/github.com/hashicorp/nomad/nomad/eval_broker.go:708 +0x3dc github.com/hashicorp/nomad/nomad.(*EvalBroker).SetEnabled() /home/schmichael/go/src/github.com/hashicorp/nomad/nomad/eval_broker.go:174 +0xc4 github.com/hashicorp/nomad/nomad.(*Server).revokeLeadership() /home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:718 +0x1fd github.com/hashicorp/nomad/nomad.(*Server).leaderLoop() /home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:122 +0x95d github.com/hashicorp/nomad/nomad.(*Server).monitorLeadership.func1() /home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:72 +0x6c Previous read at 0x00c000339150 by goroutine 73: github.com/hashicorp/nomad/nomad.(*EvalBroker).runDelayedEvalsWatcher() /home/schmichael/go/src/github.com/hashicorp/nomad/nomad/eval_broker.go:771 +0x176 Goroutine 63 (running) created at: github.com/hashicorp/nomad/nomad.(*Server).monitorLeadership() /home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:70 +0x269 Goroutine 73 (running) created at: github.com/hashicorp/nomad/nomad.(*EvalBroker).SetEnabled() /home/schmichael/go/src/github.com/hashicorp/nomad/nomad/eval_broker.go:170 +0x173 github.com/hashicorp/nomad/nomad.(*Server).establishLeadership() /home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:207 +0x355 github.com/hashicorp/nomad/nomad.(*Server).leaderLoop() /home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:117 +0x82e github.com/hashicorp/nomad/nomad.(*Server).monitorLeadership.func1() /home/schmichael/go/src/github.com/hashicorp/nomad/nomad/leader.go:72 +0x6c ================== ``` --- nomad/eval_broker.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nomad/eval_broker.go b/nomad/eval_broker.go index e507a060c..d3dfdc61e 100644 --- a/nomad/eval_broker.go +++ b/nomad/eval_broker.go @@ -10,7 +10,7 @@ import ( "context" - "github.com/armon/go-metrics" + metrics "github.com/armon/go-metrics" "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/lib/delayheap" "github.com/hashicorp/nomad/nomad/structs" @@ -167,7 +167,7 @@ func (b *EvalBroker) SetEnabled(enabled bool) { // start the go routine for delayed evals ctx, cancel := context.WithCancel(context.Background()) b.delayedEvalCancelFunc = cancel - go b.runDelayedEvalsWatcher(ctx) + go b.runDelayedEvalsWatcher(ctx, b.delayedEvalsUpdateCh) } b.l.Unlock() if !enabled { @@ -741,7 +741,7 @@ func (d *evalWrapper) Namespace() string { // runDelayedEvalsWatcher is a long-lived function that waits till a time deadline is met for // pending evaluations before enqueuing them -func (b *EvalBroker) runDelayedEvalsWatcher(ctx context.Context) { +func (b *EvalBroker) runDelayedEvalsWatcher(ctx context.Context, updateCh <-chan struct{}) { var timerChannel <-chan time.Time var delayTimer *time.Timer for { @@ -768,7 +768,7 @@ func (b *EvalBroker) runDelayedEvalsWatcher(ctx context.Context) { b.stats.TotalWaiting -= 1 b.enqueueLocked(eval, eval.Type) b.l.Unlock() - case <-b.delayedEvalsUpdateCh: + case <-updateCh: continue } }