From 6ed90bf1db701820ca394aa5d0b6f0ddcf10a265 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Wed, 2 Dec 2020 08:49:40 +0530 Subject: [PATCH] MM-30987- Fix race in setting clearNotify in Busy (#16430) A time.AfterFunc runs the function in its own goroutine. So we need to guard that with a mutex too. https://mattermost.atlassian.net/browse/MM-30987 ```release-note NONE ``` --- app/busy.go | 6 +++++- app/busy_test.go | 10 ++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/busy.go b/app/busy.go index bb1c0ceee9..3541138554 100644 --- a/app/busy.go +++ b/app/busy.go @@ -65,7 +65,11 @@ func (b *Busy) setWithoutNotify(dur time.Duration) { b.clearWithoutNotify() atomic.StoreInt32(&b.busy, 1) b.expires = time.Now().Add(dur) - b.timer = time.AfterFunc(dur, b.clearWithoutNotify) + b.timer = time.AfterFunc(dur, func() { + b.mux.Lock() + b.clearWithoutNotify() + b.mux.Unlock() + }) } // ClearBusy marks the server as not busy and notifies cluster nodes. diff --git a/app/busy_test.go b/app/busy_test.go index 3bd8db79d3..8ded6b781c 100644 --- a/app/busy_test.go +++ b/app/busy_test.go @@ -86,6 +86,16 @@ func TestBusyExpires(t *testing.T) { require.Eventually(t, func() bool { return compareBusyState(t, busy, cluster.Busy) }, time.Second*15, time.Millisecond*20) } +func TestBusyRace(t *testing.T) { + cluster := &ClusterMock{Busy: &Busy{}} + busy := NewBusy(cluster) + + busy.Set(500 * time.Millisecond) + + // We are sleeping in order to let the race trigger. + time.Sleep(time.Second) +} + func compareBusyState(t *testing.T, busy1 *Busy, busy2 *Busy) bool { t.Helper() if busy1.IsBusy() != busy2.IsBusy() {