Nulo/forgejo
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Actually recover from a panic in cron task (#28409)

Browse source 
- Currently there's code to recover gracefully from panics that happen
within the execution of cron tasks. However this recover code wasn't
being run, because `RunWithShutdownContext` also contains code to
recover from any panic and then gracefully shutdown Forgejo. Because
`RunWithShutdownContext` registers that code as last, that would get run
first which in this case is not behavior that we want.
- Move the recover code to inside the function, so that is run first
before `RunWithShutdownContext`'s recover code (which is now a noop).

Fixes: https://codeberg.org/forgejo/forgejo/issues/1910

Co-authored-by: Gusted <postmaster@gusted.xyz>
This commit is contained in:
Earl Warren 2023-12-10 21:15:06 +01:00 committed by GitHub
parent 9a99c17a14
commit fab73e4a3a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
1 changed files with 7 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
services/cron/tasks.go
View file

@ -84,13 +84,15 @@ func (t *Task) RunWithUser(doer *user_model.User, config Config) {
t.lock.Unlock() t.lock.Unlock()
defer func() { defer func() {
taskStatusTable.Stop(t.Name) taskStatusTable.Stop(t.Name)
}()
graceful.GetManager().RunWithShutdownContext(func(baseCtx context.Context) {
defer func() {
if err := recover(); err != nil { if err := recover(); err != nil {
// Recover a panic within the // Recover a panic within the execution of the task.
combinedErr := fmt.Errorf("%s\n%s", err, log.Stack(2)) combinedErr := fmt.Errorf("%s\n%s", err, log.Stack(2))
log.Error("PANIC whilst running task: %s Value: %v", t.Name, combinedErr) log.Error("PANIC whilst running task: %s Value: %v", t.Name, combinedErr)
} }
}() }()
graceful.GetManager().RunWithShutdownContext(func(baseCtx context.Context) {
// Store the time of this run, before the function is executed, so it // Store the time of this run, before the function is executed, so it
// matches the behavior of what the cron library does. // matches the behavior of what the cron library does.
t.lock.Lock() t.lock.Lock()