From ff4406e8b2be1d5b42dfb73cbae18c87b57cb5e1 Mon Sep 17 00:00:00 2001 From: Ashish Bhate Date: Fri, 19 Aug 2022 07:27:16 +0530 Subject: [PATCH] MM-46402: Use pointer to pointer to set pointer to nil (#20827) Summary We need a pointer to a pointer to set the original pointer to nil The original task was not being set to nil (the local variable containing the pointer was being set to nil). The cancel function was being called even though the task had already been cancelled. This repeated cancellation was causing a panic because we were trying to close a channel that had already been closed the first time the task was cancelled. Ticket Link https://mattermost.atlassian.net/browse/MM-46402 --- app/server.go | 12 ++++++------ app/server_test.go | 10 ++++++++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/app/server.go b/app/server.go index f1477d0129..e00d8743c9 100644 --- a/app/server.go +++ b/app/server.go @@ -2060,12 +2060,12 @@ func withMut(mut *sync.Mutex, f func()) { f() } -func cancelTask(mut *sync.Mutex, task *model.ScheduledTask) { +func cancelTask(mut *sync.Mutex, taskPointer **model.ScheduledTask) { mut.Lock() defer mut.Unlock() - if task != nil { - task.Cancel() - task = nil + if *taskPointer != nil { + (*taskPointer).Cancel() + *taskPointer = nil } } @@ -2082,7 +2082,7 @@ func runDNDStatusExpireJob(a *App) { a.ch.dndTask = model.CreateRecurringTaskFromNextIntervalTime("Unset DND Statuses", a.UpdateDNDStatusOfUsers, 5*time.Minute) }) } else { - cancelTask(&a.ch.dndTaskMut, a.ch.dndTask) + cancelTask(&a.ch.dndTaskMut, &a.ch.dndTask) } }) } @@ -2100,7 +2100,7 @@ func runPostReminderJob(a *App) { a.ch.postReminderTask = model.CreateRecurringTaskFromNextIntervalTime("Check Post reminders", a.CheckPostReminders, 5*time.Minute) }) } else { - cancelTask(&a.ch.postReminderMut, a.ch.postReminderTask) + cancelTask(&a.ch.postReminderMut, &a.ch.postReminderTask) } }) } diff --git a/app/server_test.go b/app/server_test.go index c1e0cd8fec..496a10822b 100644 --- a/app/server_test.go +++ b/app/server_test.go @@ -16,6 +16,7 @@ import ( "path" "strconv" "strings" + "sync" "testing" "time" @@ -565,3 +566,12 @@ func TestSentry(t *testing.T) { } }) } + +func TestCancelTaskSetsTaskToNil(t *testing.T) { + var taskMut sync.Mutex + task := model.CreateRecurringTaskFromNextIntervalTime("a test task", func() {}, 5*time.Minute) + require.NotNil(t, task) + cancelTask(&taskMut, &task) + require.Nil(t, task) + require.NotPanics(t, func() { cancelTask(&taskMut, &task) }) +}