From a20295c65b9bed707944b305f8e3c5239d2f6b5c Mon Sep 17 00:00:00 2001 From: Anthony Romano Date: Fri, 14 Apr 2017 09:50:33 -0700 Subject: [PATCH] auth: fix race on stopping simple token keeper run goroutine was resetting a field for no reason and without holding a lock. This patch cleans up the run goroutine management to make the start/stop path less racey in general. --- auth/simple_token.go | 24 ++++++++++++++---------- auth/store.go | 7 ++++--- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/auth/simple_token.go b/auth/simple_token.go index 3a60fc44a..a39f39276 100644 --- a/auth/simple_token.go +++ b/auth/simple_token.go @@ -38,16 +38,18 @@ var ( type simpleTokenTTLKeeper struct { tokens map[string]time.Time - stopCh chan chan struct{} + donec chan struct{} + stopc chan struct{} deleteTokenFunc func(string) mu *sync.Mutex } func (tm *simpleTokenTTLKeeper) stop() { - waitCh := make(chan struct{}) - tm.stopCh <- waitCh - <-waitCh - close(tm.stopCh) + select { + case tm.stopc <- struct{}{}: + case <-tm.donec: + } + <-tm.donec } func (tm *simpleTokenTTLKeeper) addSimpleToken(token string) { @@ -66,7 +68,10 @@ func (tm *simpleTokenTTLKeeper) deleteSimpleToken(token string) { func (tm *simpleTokenTTLKeeper) run() { tokenTicker := time.NewTicker(simpleTokenTTLResolution) - defer tokenTicker.Stop() + defer func() { + tokenTicker.Stop() + close(tm.donec) + }() for { select { case <-tokenTicker.C: @@ -79,9 +84,7 @@ func (tm *simpleTokenTTLKeeper) run() { } } tm.mu.Unlock() - case waitCh := <-tm.stopCh: - tm.tokens = make(map[string]time.Time) - waitCh <- struct{}{} + case <-tm.stopc: return } } @@ -96,7 +99,8 @@ func (as *authStore) enable() { } as.simpleTokenKeeper = &simpleTokenTTLKeeper{ tokens: make(map[string]time.Time), - stopCh: make(chan chan struct{}), + donec: make(chan struct{}), + stopc: make(chan struct{}), deleteTokenFunc: delf, mu: &as.simpleTokensMu, } diff --git a/auth/store.go b/auth/store.go index 221d5cb7a..236bb2c52 100644 --- a/auth/store.go +++ b/auth/store.go @@ -243,11 +243,12 @@ func (as *authStore) AuthDisable() { as.enabled = false as.simpleTokensMu.Lock() + tk := as.simpleTokenKeeper + as.simpleTokenKeeper = nil as.simpleTokens = make(map[string]string) // invalidate all tokens as.simpleTokensMu.Unlock() - if as.simpleTokenKeeper != nil { - as.simpleTokenKeeper.stop() - as.simpleTokenKeeper = nil + if tk != nil { + tk.stop() } plog.Noticef("Authentication disabled")