Skip to content

Commit

Permalink
MM-46402: Use pointer to pointer to set pointer to nil (mattermost#20827
Browse files Browse the repository at this point in the history
)

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
  • Loading branch information
Ashish Bhate committed Aug 19, 2022
1 parent 9de6ce5 commit ff4406e
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 6 deletions.
12 changes: 6 additions & 6 deletions app/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand All @@ -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)
}
})
}
Expand All @@ -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)
}
})
}
Expand Down
10 changes: 10 additions & 0 deletions app/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"path"
"strconv"
"strings"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -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) })
}

0 comments on commit ff4406e

Please sign in to comment.