Handle panics that percolate up to the graceful module (go-gitea#11291)

* Handle panics in graceful goroutines Adds a some deferred functions to handle panics in graceful goroutines Signed-off-by: Andrew Thornton <[email protected]> * Handle panic in webhook.Deliver Signed-off-by: Andrew Thornton <[email protected]> * Handle panic in mirror.syncMirror Signed-off-by: Andrew Thornton <[email protected]> Co-authored-by: Lauris BH <[email protected]> Co-authored-by: techknowlogick <[email protected]>
ydelafollye · Jul 31, 2020 · 4c016d5 · 4c016d5
1 parent 036bc26
commit 4c016d5
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 1 deletion.
diff --git a/modules/graceful/manager.go b/modules/graceful/manager.go
@@ -74,6 +74,12 @@ type RunnableWithShutdownFns func(atShutdown, atTerminate func(context.Context,
 func (g *Manager) RunWithShutdownFns(run RunnableWithShutdownFns) {
  g.runningServerWaitGroup.Add(1)
  defer g.runningServerWaitGroup.Done()
+ defer func() {
+ if err := recover(); err != nil {
+ log.Critical("PANIC during RunWithShutdownFns: %v\nStacktrace: %s", err, log.Stack(2))
+ g.doShutdown()
+ }
+ }()
  run(func(ctx context.Context, atShutdown func()) {
  go func() {
  select {
@@ -103,6 +109,12 @@ type RunnableWithShutdownChan func(atShutdown <-chan struct{}, atTerminate Callb
 func (g *Manager) RunWithShutdownChan(run RunnableWithShutdownChan) {
  g.runningServerWaitGroup.Add(1)
  defer g.runningServerWaitGroup.Done()
+ defer func() {
+ if err := recover(); err != nil {
+ log.Critical("PANIC during RunWithShutdownChan: %v\nStacktrace: %s", err, log.Stack(2))
+ g.doShutdown()
+ }
+ }()
  run(g.IsShutdown(), func(ctx context.Context, atTerminate func()) {
  g.RunAtTerminate(ctx, atTerminate)
  })
@@ -114,25 +126,41 @@ func (g *Manager) RunWithShutdownChan(run RunnableWithShutdownChan) {
 func (g *Manager) RunWithShutdownContext(run func(context.Context)) {
  g.runningServerWaitGroup.Add(1)
  defer g.runningServerWaitGroup.Done()
+ defer func() {
+ if err := recover(); err != nil {
+ log.Critical("PANIC during RunWithShutdownContext: %v\nStacktrace: %s", err, log.Stack(2))
+ g.doShutdown()
+ }
+ }()
  run(g.ShutdownContext())
 }
 
 // RunAtTerminate adds to the terminate wait group and creates a go-routine to run the provided function at termination
 func (g *Manager) RunAtTerminate(ctx context.Context, terminate func()) {
  g.terminateWaitGroup.Add(1)
  go func() {
+ defer g.terminateWaitGroup.Done()
+ defer func() {
+ if err := recover(); err != nil {
+ log.Critical("PANIC during RunAtTerminate: %v\nStacktrace: %s", err, log.Stack(2))
+ }
+ }()
  select {
  case <-g.IsTerminate():
  terminate()
  case <-ctx.Done():
  }
- g.terminateWaitGroup.Done()
  }()
 }
 
 // RunAtShutdown creates a go-routine to run the provided function at shutdown
 func (g *Manager) RunAtShutdown(ctx context.Context, shutdown func()) {
  go func() {
+ defer func() {
+ if err := recover(); err != nil {
+ log.Critical("PANIC during RunAtShutdown: %v\nStacktrace: %s", err, log.Stack(2))
+ }
+ }()
  select {
  case <-g.IsShutdown():
  shutdown()
@@ -144,6 +172,11 @@ func (g *Manager) RunAtShutdown(ctx context.Context, shutdown func()) {
 // RunAtHammer creates a go-routine to run the provided function at shutdown
 func (g *Manager) RunAtHammer(ctx context.Context, hammer func()) {
  go func() {
+ defer func() {
+ if err := recover(); err != nil {
+ log.Critical("PANIC during RunAtHammer: %v\nStacktrace: %s", err, log.Stack(2))
+ }
+ }()
  select {
  case <-g.IsHammer():
  hammer()

diff --git a/modules/webhook/deliver.go b/modules/webhook/deliver.go
@@ -26,6 +26,14 @@ import (
 
 // Deliver deliver hook task
 func Deliver(t *models.HookTask) error {
+ defer func() {
+ err := recover()
+ if err == nil {
+ return
+ }
+ // There was a panic whilst delivering a hook...
+ log.Error("PANIC whilst trying to deliver webhook[%d] for repo[%d] to %s Panic: %v\nStacktrace: %s", t.ID, t.RepoID, t.URL, err, log.Stack(2))
+ }()
  t.IsDelivered = true
 
  var req *http.Request

diff --git a/services/mirror/mirror.go b/services/mirror/mirror.go
@@ -333,6 +333,14 @@ func SyncMirrors(ctx context.Context) {
 
 func syncMirror(repoID string) {
  log.Trace("SyncMirrors [repo_id: %v]", repoID)
+ defer func() {
+ err := recover()
+ if err == nil {
+ return
+ }
+ // There was a panic whilst syncMirrors...
+ log.Error("PANIC whilst syncMirrors[%s] Panic: %v\nStacktrace: %s", repoID, err, log.Stack(2))
+ }()
  mirrorQueue.Remove(repoID)
 
  m, err := models.GetMirrorByRepoID(com.StrTo(repoID).MustInt64())