From c58bc4bf804a3e8f92dd634974ed4f636893c9c1 Mon Sep 17 00:00:00 2001
From: zeripath <art27@cantab.net>
Date: Fri, 8 May 2020 16:46:05 +0100
Subject: [PATCH] Prevent timer leaks in Workerpool and others (#11333)

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>
---
 cmd/hook.go                    | 12 +++---------
 modules/migrations/github.go   |  3 ++-
 modules/queue/queue_wrapped.go |  3 ++-
 modules/queue/workerpool.go    | 22 ++++------------------
 modules/util/timer.go          | 21 +++++++++++++++++++++
 5 files changed, 32 insertions(+), 29 deletions(-)
 create mode 100644 modules/util/timer.go

diff --git a/cmd/hook.go b/cmd/hook.go
index 331f6a2d2..fa932087f 100644
--- a/cmd/hook.go
+++ b/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()
-	if stopped {
-		return nil
-	}
-	select {
-	case <-d.timer.C:
-	default:
-	}
-	if d.buf == nil {
+	stopped := util.StopTimer(d.timer)
+	if stopped || d.buf == nil {
 		return nil
 	}
 	_, err := d.internal.Write(d.buf.Bytes())
diff --git a/modules/migrations/github.go b/modules/migrations/github.go
index 488cd82f5..b6bdab09f 100644
--- a/modules/migrations/github.go
+++ b/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:
 		}
diff --git a/modules/queue/queue_wrapped.go b/modules/queue/queue_wrapped.go
index 482c90a81..88d64e824 100644
--- a/modules/queue/queue_wrapped.go
+++ b/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
@@ -88,7 +89,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:
 			}
 		}
diff --git a/modules/queue/workerpool.go b/modules/queue/workerpool.go
index 952e87681..45378e3da 100644
--- a/modules/queue/workerpool.go
+++ b/modules/queue/workerpool.go
@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/util"
 )
 
 // WorkerPool represent a dynamically growable worker pool for a
@@ -92,12 +93,7 @@ func (p *WorkerPool) pushBoost(data Data) {
 		p.lock.Unlock()
 		select {
 		case p.dataChan <- data:
-			if timer.Stop() {
-				select {
-				case <-timer.C:
-				default:
-				}
-			}
+			util.StopTimer(timer)
 		case <-timer.C:
 			p.lock.Lock()
 			if p.blockTimeout > ourTimeout || (p.numberOfWorkers > p.maxNumberOfWorkers && p.maxNumberOfWorkers >= 0) {
@@ -353,12 +349,7 @@ func (p *WorkerPool) doWork(ctx context.Context) {
 			timer := time.NewTimer(delay)
 			select {
 			case <-ctx.Done():
-				if timer.Stop() {
-					select {
-					case <-timer.C:
-					default:
-					}
-				}
+				util.StopTimer(timer)
 				if len(data) > 0 {
 					log.Trace("Handling: %d data, %v", len(data), data)
 					p.handle(data...)
@@ -367,12 +358,7 @@ func (p *WorkerPool) doWork(ctx context.Context) {
 				log.Trace("Worker shutting down")
 				return
 			case datum, ok := <-p.dataChan:
-				if timer.Stop() {
-					select {
-					case <-timer.C:
-					default:
-					}
-				}
+				util.StopTimer(timer)
 				if !ok {
 					// the dataChan has been closed - we should finish up:
 					if len(data) > 0 {
diff --git a/modules/util/timer.go b/modules/util/timer.go
new file mode 100644
index 000000000..b682f376d
--- /dev/null
+++ b/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
+}