Prevent timer leaks in Workerpool and others (#11333) (#11340)

There is a potential memory leak in `Workerpool` due to the intricacies of
`time.Timer` stopping.

Whenever a `time.Timer` is `Stop`ped its channel must be cleared using a
`select` if the result of the `Stop()` is `false`.

Unfortunately in `Workerpool` these were checked the wrong way round.

However, there were a few other places that were not being checked.

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

cmd/hook.go

@@ -19,6 +19,7 @@ import (
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/private"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/util"
 
 	"github.com/urfave/cli"
 )
@@ -113,15 +114,8 @@ func (d *delayWriter) Close() error {
 	if d == nil {
 		return nil
 	}
-	stopped := d.timer.Stop()
+	stopped := util.StopTimer(d.timer)
-	if stopped {
+	if stopped || d.buf == nil {
-		return nil
-	}
-	select {
-	case <-d.timer.C:
-	default:
-	}
-	if d.buf == nil {
 		return nil
 	}
 	_, err := d.internal.Write(d.buf.Bytes())

modules/migrations/github.go

@@ -16,6 +16,7 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/migrations/base"
 	"code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/util"
 
 	"github.com/google/go-github/v24/github"
 	"golang.org/x/oauth2"
@@ -121,7 +122,7 @@ func (g *GithubDownloaderV3) sleep() {
 		timer := time.NewTimer(time.Until(g.rate.Reset.Time))
 		select {
 		case <-g.ctx.Done():
-			timer.Stop()
+			util.StopTimer(timer)
 			return
 		case <-timer.C:
 		}

modules/queue/queue_wrapped.go

@@ -12,6 +12,7 @@ import (
 	"time"
 
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/util"
 )
 
 // WrappedQueueType is the type for a wrapped delayed starting queue
@@ -77,7 +78,7 @@ func (q *delayedStarter) setInternal(atShutdown func(context.Context, func()), h
 			t := time.NewTimer(sleepTime)
 			select {
 			case <-ctx.Done():
-				t.Stop()
+				util.StopTimer(t)
 			case <-t.C:
 			}
 		}

modules/queue/workerpool.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/util"
 )
 
 // WorkerPool takes
@@ -56,12 +57,7 @@ func (p *WorkerPool) pushBoost(data Data) {
 		p.lock.Unlock()
 		select {
 		case p.dataChan <- data:
-			if timer.Stop() {
+			util.StopTimer(timer)
-				select {
-				case <-timer.C:
-				default:
-				}
-			}
 		case <-timer.C:
 			p.lock.Lock()
 			if p.blockTimeout > ourTimeout || (p.numberOfWorkers > p.maxNumberOfWorkers && p.maxNumberOfWorkers >= 0) {
@@ -277,12 +273,7 @@ func (p *WorkerPool) doWork(ctx context.Context) {
 			timer := time.NewTimer(delay)
 			select {
 			case <-ctx.Done():
-				if timer.Stop() {
+				util.StopTimer(timer)
-					select {
-					case <-timer.C:
-					default:
-					}
-				}
 				if len(data) > 0 {
 					log.Trace("Handling: %d data, %v", len(data), data)
 					p.handle(data...)
@@ -290,12 +281,7 @@ func (p *WorkerPool) doWork(ctx context.Context) {
 				log.Trace("Worker shutting down")
 				return
 			case datum, ok := <-p.dataChan:
-				if timer.Stop() {
+				util.StopTimer(timer)
-					select {
-					case <-timer.C:
-					default:
-					}
-				}
 				if !ok {
 					// the dataChan has been closed - we should finish up:
 					if len(data) > 0 {

modules/util/timer.go

@@ -0,0 +1,21 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package util
+
+import (
+	"time"
+)
+
+// StopTimer is a utility function to safely stop a time.Timer and clean its channel
+func StopTimer(t *time.Timer) bool {
+	stopped := t.Stop()
+	if !stopped {
+		select {
+		case <-t.C:
+		default:
+		}
+	}
+	return stopped
+}