From d6f6519fe61b3c886eeed031fb30a9699c2cc30c Mon Sep 17 00:00:00 2001 From: Aman Mangal Date: Fri, 3 Apr 2020 21:55:58 +0530 Subject: [PATCH] Fix panic in Task FrameWork, fixes #5034 (#5081) (cherry-picked from commit 8482527535617914c4c52db79b312f5b21543455) --- worker/draft.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/worker/draft.go b/worker/draft.go index 53b9e4a3cac..1cccb4aaa64 100644 --- a/worker/draft.go +++ b/worker/draft.go @@ -95,6 +95,9 @@ const ( // startTask is used to check whether an op is already going on. // If rollup is going on, we cancel and wait for rollup to complete // before we return. If the same task is already going, we return error. +// You should only call Done() on the returned closer. Calling other +// functions (such as SignalAndWait) for closer could result in panics. +// For more details, see GitHub issue #5034. func (n *node) startTask(id op) (*y.Closer, error) { n.opsLock.Lock() defer n.opsLock.Unlock() @@ -125,6 +128,8 @@ func (n *node) startTask(id op) (*y.Closer, error) { case opSnapshot, opIndexing: for otherId, otherCloser := range n.ops { if otherId == opRollup { + // We set to nil so that stopAllTasks doesn't call SignalAndWait again. + n.ops[opRollup] = nil otherCloser.SignalAndWait() } else { return nil, errors.Errorf("operation %s is already running", otherId) @@ -158,14 +163,12 @@ func (n *node) stopAllTasks() { defer n.closer.Done() // CLOSER:1 <-n.closer.HasBeenClosed() - var closers []*y.Closer n.opsLock.Lock() + defer n.opsLock.Unlock() for _, closer := range n.ops { - closers = append(closers, closer) - } - n.opsLock.Unlock() - - for _, closer := range closers { + if closer == nil { + continue + } closer.SignalAndWait() } glog.Infof("Stopped all ongoing registered tasks.")