foster/forgejo
1
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2024-11-27 09:11:53 -05:00
Code Issues Wiki Activity

Recover from panic in cron task (#28409) (#28425)

Browse source 
Backport #28409 by @earl-warren

- 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: Earl Warren <109468362+earl-warren@users.noreply.github.com>
Co-authored-by: Gusted <postmaster@gusted.xyz>
This commit is contained in:
Giteabot 2023-12-12 11:28:56 +08:00 committed by GitHub
parent 1ec622db24
commit 6f4d5c0b8c
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

12
services/cron/tasks.go
View file

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