From 4a0c4c48470a5d72807c8f9e714921b8342023e5 Mon Sep 17 00:00:00 2001
From: SmallCoccinelle <89733524+SmallCoccinelle@users.noreply.github.com>
Date: Wed, 22 Sep 2021 05:22:59 +0200
Subject: [PATCH] Reorder waitgroup completion (#1748)

Rather than passing a pointer to a waitgroup into task.Start(..)
functions, handle the waitgroup.Done() at the callsite.

This makes waitgroup handling local to its definition rather than it
being spread out over multiple files. Tasks now simply execute, and
the policy of waiting on them is handled by the caller.
---
 pkg/manager/manager_tasks.go            | 39 +++++++++++++++----------
 pkg/manager/task.go                     |  4 +--
 pkg/manager/task_generate_markers.go    |  6 +---
 pkg/manager/task_generate_phash.go      |  6 +---
 pkg/manager/task_generate_preview.go    |  6 +---
 pkg/manager/task_generate_screenshot.go |  5 +---
 pkg/manager/task_generate_sprite.go     |  6 +---
 pkg/manager/task_import.go              |  5 +---
 pkg/manager/task_migrate_hash.go        |  6 +---
 pkg/manager/task_scan.go                | 20 ++++++-------
 pkg/manager/task_stash_box_tag.go       |  5 +---
 pkg/manager/task_transcode.go           |  6 +---
 12 files changed, 43 insertions(+), 71 deletions(-)

diff --git a/pkg/manager/manager_tasks.go b/pkg/manager/manager_tasks.go
index 56c16a352..345c06f18 100644
--- a/pkg/manager/manager_tasks.go
+++ b/pkg/manager/manager_tasks.go
@@ -82,9 +82,6 @@ func (s *singleton) Import(ctx context.Context) (int, error) {
 	}
 
 	j := job.MakeJobExec(func(ctx context.Context, progress *job.Progress) {
-		var wg sync.WaitGroup
-		wg.Add(1)
-
 		task := ImportTask{
 			txnManager:          s.TxnManager,
 			BaseDir:             metadataPath,
@@ -93,7 +90,7 @@ func (s *singleton) Import(ctx context.Context) (int, error) {
 			MissingRefBehaviour: models.ImportMissingRefEnumFail,
 			fileNamingAlgorithm: config.GetVideoFileNamingAlgorithm(),
 		}
-		task.Start(&wg)
+		task.Start()
 	})
 
 	return s.JobManager.Add(ctx, "Importing...", j), nil
@@ -125,7 +122,8 @@ func (s *singleton) RunSingleTask(ctx context.Context, t Task) int {
 	wg.Add(1)
 
 	j := job.MakeJobExec(func(ctx context.Context, progress *job.Progress) {
-		t.Start(&wg)
+		t.Start()
+		wg.Done()
 	})
 
 	return s.JobManager.Add(ctx, t.GetDescription(), j)
@@ -280,7 +278,8 @@ func (s *singleton) Generate(ctx context.Context, input models.GenerateMetadataI
 				}
 				wg.Add()
 				go progress.ExecuteTask(fmt.Sprintf("Generating sprites for %s", scene.Path), func() {
-					task.Start(&wg)
+					task.Start()
+					wg.Done()
 				})
 			}
 
@@ -294,7 +293,8 @@ func (s *singleton) Generate(ctx context.Context, input models.GenerateMetadataI
 				}
 				wg.Add()
 				go progress.ExecuteTask(fmt.Sprintf("Generating preview for %s", scene.Path), func() {
-					task.Start(&wg)
+					task.Start()
+					wg.Done()
 				})
 			}
 
@@ -309,7 +309,8 @@ func (s *singleton) Generate(ctx context.Context, input models.GenerateMetadataI
 					Screenshot:          input.MarkerScreenshots,
 				}
 				go progress.ExecuteTask(fmt.Sprintf("Generating markers for %s", scene.Path), func() {
-					task.Start(&wg)
+					task.Start()
+					wg.Done()
 				})
 			}
 
@@ -321,7 +322,8 @@ func (s *singleton) Generate(ctx context.Context, input models.GenerateMetadataI
 					fileNamingAlgorithm: fileNamingAlgo,
 				}
 				go progress.ExecuteTask(fmt.Sprintf("Generating transcode for %s", scene.Path), func() {
-					task.Start(&wg)
+					task.Start()
+					wg.Done()
 				})
 			}
 
@@ -334,7 +336,8 @@ func (s *singleton) Generate(ctx context.Context, input models.GenerateMetadataI
 				}
 				wg.Add()
 				go progress.ExecuteTask(fmt.Sprintf("Generating phash for %s", scene.Path), func() {
-					task.Start(&wg)
+					task.Start()
+					wg.Done()
 				})
 			}
 		}
@@ -367,7 +370,8 @@ func (s *singleton) Generate(ctx context.Context, input models.GenerateMetadataI
 				fileNamingAlgorithm: fileNamingAlgo,
 			}
 			go progress.ExecuteTask(fmt.Sprintf("Generating marker preview for marker ID %d", marker.ID), func() {
-				task.Start(&wg)
+				task.Start()
+				wg.Done()
 			})
 		}
 
@@ -421,9 +425,7 @@ func (s *singleton) generateScreenshot(ctx context.Context, sceneId string, at *
 			fileNamingAlgorithm: config.GetInstance().GetVideoFileNamingAlgorithm(),
 		}
 
-		var wg sync.WaitGroup
-		wg.Add(1)
-		task.Start(&wg)
+		task.Start()
 
 		logger.Infof("Generate screenshot finished")
 	})
@@ -607,7 +609,11 @@ func (s *singleton) MigrateHash(ctx context.Context) int {
 			wg.Add(1)
 
 			task := MigrateHashTask{Scene: scene, fileNamingAlgorithm: fileNamingAlgo}
-			go task.Start(&wg)
+			go func() {
+				task.Start()
+				wg.Done()
+			}()
+
 			wg.Wait()
 		}
 
@@ -811,7 +817,8 @@ func (s *singleton) StashBoxBatchPerformerTag(ctx context.Context, input models.
 		for _, task := range tasks {
 			wg.Add(1)
 			progress.ExecuteTask(task.Description(), func() {
-				task.Start(&wg)
+				task.Start()
+				wg.Done()
 			})
 
 			progress.Increment()
diff --git a/pkg/manager/task.go b/pkg/manager/task.go
index c8ab4af67..9906948c8 100644
--- a/pkg/manager/task.go
+++ b/pkg/manager/task.go
@@ -1,8 +1,6 @@
 package manager
 
-import "sync"
-
 type Task interface {
-	Start(wg *sync.WaitGroup)
+	Start()
 	GetDescription() string
 }
diff --git a/pkg/manager/task_generate_markers.go b/pkg/manager/task_generate_markers.go
index 5c432bf3a..153fb3517 100644
--- a/pkg/manager/task_generate_markers.go
+++ b/pkg/manager/task_generate_markers.go
@@ -5,8 +5,6 @@ import (
 	"path/filepath"
 	"strconv"
 
-	"github.com/remeh/sizedwaitgroup"
-
 	"github.com/stashapp/stash/pkg/ffmpeg"
 	"github.com/stashapp/stash/pkg/logger"
 	"github.com/stashapp/stash/pkg/models"
@@ -24,9 +22,7 @@ type GenerateMarkersTask struct {
 	Screenshot   bool
 }
 
-func (t *GenerateMarkersTask) Start(wg *sizedwaitgroup.SizedWaitGroup) {
-	defer wg.Done()
-
+func (t *GenerateMarkersTask) Start() {
 	if t.Scene != nil {
 		t.generateSceneMarkers()
 	}
diff --git a/pkg/manager/task_generate_phash.go b/pkg/manager/task_generate_phash.go
index 0c1578ee5..9d2fa172f 100644
--- a/pkg/manager/task_generate_phash.go
+++ b/pkg/manager/task_generate_phash.go
@@ -1,8 +1,6 @@
 package manager
 
 import (
-	"github.com/remeh/sizedwaitgroup"
-
 	"context"
 	"database/sql"
 
@@ -18,9 +16,7 @@ type GeneratePhashTask struct {
 	txnManager          models.TransactionManager
 }
 
-func (t *GeneratePhashTask) Start(wg *sizedwaitgroup.SizedWaitGroup) {
-	defer wg.Done()
-
+func (t *GeneratePhashTask) Start() {
 	if !t.shouldGenerate() {
 		return
 	}
diff --git a/pkg/manager/task_generate_preview.go b/pkg/manager/task_generate_preview.go
index c5c666a6d..01a68f006 100644
--- a/pkg/manager/task_generate_preview.go
+++ b/pkg/manager/task_generate_preview.go
@@ -1,8 +1,6 @@
 package manager
 
 import (
-	"github.com/remeh/sizedwaitgroup"
-
 	"github.com/stashapp/stash/pkg/ffmpeg"
 	"github.com/stashapp/stash/pkg/logger"
 	"github.com/stashapp/stash/pkg/manager/config"
@@ -20,9 +18,7 @@ type GeneratePreviewTask struct {
 	fileNamingAlgorithm models.HashAlgorithm
 }
 
-func (t *GeneratePreviewTask) Start(wg *sizedwaitgroup.SizedWaitGroup) {
-	defer wg.Done()
-
+func (t *GeneratePreviewTask) Start() {
 	videoFilename := t.videoFilename()
 	videoChecksum := t.Scene.GetHash(t.fileNamingAlgorithm)
 	imageFilename := t.imageFilename()
diff --git a/pkg/manager/task_generate_screenshot.go b/pkg/manager/task_generate_screenshot.go
index 3be100c86..ddb81fc40 100644
--- a/pkg/manager/task_generate_screenshot.go
+++ b/pkg/manager/task_generate_screenshot.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
-	"sync"
 	"time"
 
 	"github.com/stashapp/stash/pkg/ffmpeg"
@@ -20,9 +19,7 @@ type GenerateScreenshotTask struct {
 	txnManager          models.TransactionManager
 }
 
-func (t *GenerateScreenshotTask) Start(wg *sync.WaitGroup) {
-	defer wg.Done()
-
+func (t *GenerateScreenshotTask) Start() {
 	scenePath := t.Scene.Path
 	probeResult, err := ffmpeg.NewVideoFile(instance.FFProbePath, scenePath, false)
 
diff --git a/pkg/manager/task_generate_sprite.go b/pkg/manager/task_generate_sprite.go
index 20caeb78c..0a21b6011 100644
--- a/pkg/manager/task_generate_sprite.go
+++ b/pkg/manager/task_generate_sprite.go
@@ -1,8 +1,6 @@
 package manager
 
 import (
-	"github.com/remeh/sizedwaitgroup"
-
 	"github.com/stashapp/stash/pkg/ffmpeg"
 	"github.com/stashapp/stash/pkg/logger"
 	"github.com/stashapp/stash/pkg/models"
@@ -15,9 +13,7 @@ type GenerateSpriteTask struct {
 	fileNamingAlgorithm models.HashAlgorithm
 }
 
-func (t *GenerateSpriteTask) Start(wg *sizedwaitgroup.SizedWaitGroup) {
-	defer wg.Done()
-
+func (t *GenerateSpriteTask) Start() {
 	if !t.Overwrite && !t.required() {
 		return
 	}
diff --git a/pkg/manager/task_import.go b/pkg/manager/task_import.go
index 374c61296..4d9b535ae 100644
--- a/pkg/manager/task_import.go
+++ b/pkg/manager/task_import.go
@@ -8,7 +8,6 @@ import (
 	"io"
 	"os"
 	"path/filepath"
-	"sync"
 	"time"
 
 	"github.com/stashapp/stash/pkg/database"
@@ -79,9 +78,7 @@ func (t *ImportTask) GetDescription() string {
 	return "Importing..."
 }
 
-func (t *ImportTask) Start(wg *sync.WaitGroup) {
-	defer wg.Done()
-
+func (t *ImportTask) Start() {
 	if t.TmpZip != "" {
 		defer func() {
 			err := utils.RemoveDir(t.BaseDir)
diff --git a/pkg/manager/task_migrate_hash.go b/pkg/manager/task_migrate_hash.go
index 3305ad9f6..3ecdb54d4 100644
--- a/pkg/manager/task_migrate_hash.go
+++ b/pkg/manager/task_migrate_hash.go
@@ -1,8 +1,6 @@
 package manager
 
 import (
-	"sync"
-
 	"github.com/stashapp/stash/pkg/models"
 )
 
@@ -14,9 +12,7 @@ type MigrateHashTask struct {
 }
 
 // Start starts the task.
-func (t *MigrateHashTask) Start(wg *sync.WaitGroup) {
-	defer wg.Done()
-
+func (t *MigrateHashTask) Start() {
 	if !t.Scene.OSHash.Valid || !t.Scene.Checksum.Valid {
 		// nothing to do
 		return
diff --git a/pkg/manager/task_scan.go b/pkg/manager/task_scan.go
index 34f7c2208..47146847a 100644
--- a/pkg/manager/task_scan.go
+++ b/pkg/manager/task_scan.go
@@ -109,7 +109,8 @@ func (j *ScanJob) Execute(ctx context.Context, progress *job.Progress) {
 			}
 
 			go func() {
-				task.Start(&wg)
+				task.Start()
+				wg.Done()
 				progress.Increment()
 			}()
 
@@ -225,9 +226,7 @@ type ScanTask struct {
 	CaseSensitiveFs      bool
 }
 
-func (t *ScanTask) Start(wg *sizedwaitgroup.SizedWaitGroup) {
-	defer wg.Done()
-
+func (t *ScanTask) Start() {
 	var s *models.Scene
 
 	t.progress.ExecuteTask("Scanning "+t.FilePath, func() {
@@ -252,7 +251,8 @@ func (t *ScanTask) Start(wg *sizedwaitgroup.SizedWaitGroup) {
 					Overwrite:           false,
 					fileNamingAlgorithm: t.fileNamingAlgorithm,
 				}
-				taskSprite.Start(&iwg)
+				taskSprite.Start()
+				iwg.Done()
 			})
 		}
 
@@ -265,7 +265,8 @@ func (t *ScanTask) Start(wg *sizedwaitgroup.SizedWaitGroup) {
 					fileNamingAlgorithm: t.fileNamingAlgorithm,
 					txnManager:          t.TxnManager,
 				}
-				taskPhash.Start(&iwg)
+				taskPhash.Start()
+				iwg.Done()
 			})
 		}
 
@@ -296,7 +297,8 @@ func (t *ScanTask) Start(wg *sizedwaitgroup.SizedWaitGroup) {
 					Overwrite:           false,
 					fileNamingAlgorithm: t.fileNamingAlgorithm,
 				}
-				taskPreview.Start(wg)
+				taskPreview.Start()
+				iwg.Done()
 			})
 		}
 
@@ -972,9 +974,7 @@ func (t *ScanTask) scanZipImages(zipGallery *models.Gallery) {
 		subTask.zipGallery = zipGallery
 
 		// run the subtask and wait for it to complete
-		iwg := sizedwaitgroup.New(1)
-		iwg.Add()
-		subTask.Start(&iwg)
+		subTask.Start()
 		return nil
 	})
 	if err != nil {
diff --git a/pkg/manager/task_stash_box_tag.go b/pkg/manager/task_stash_box_tag.go
index 8429fb2ee..dd15e7808 100644
--- a/pkg/manager/task_stash_box_tag.go
+++ b/pkg/manager/task_stash_box_tag.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"database/sql"
 	"fmt"
-	"sync"
 	"time"
 
 	"github.com/stashapp/stash/pkg/logger"
@@ -22,9 +21,7 @@ type StashBoxPerformerTagTask struct {
 	excluded_fields []string
 }
 
-func (t *StashBoxPerformerTagTask) Start(wg *sync.WaitGroup) {
-	defer wg.Done()
-
+func (t *StashBoxPerformerTagTask) Start() {
 	t.stashBoxPerformerTag()
 }
 
diff --git a/pkg/manager/task_transcode.go b/pkg/manager/task_transcode.go
index 5a9161966..7c55eaba5 100644
--- a/pkg/manager/task_transcode.go
+++ b/pkg/manager/task_transcode.go
@@ -1,8 +1,6 @@
 package manager
 
 import (
-	"github.com/remeh/sizedwaitgroup"
-
 	"github.com/stashapp/stash/pkg/ffmpeg"
 	"github.com/stashapp/stash/pkg/logger"
 	"github.com/stashapp/stash/pkg/manager/config"
@@ -16,9 +14,7 @@ type GenerateTranscodeTask struct {
 	fileNamingAlgorithm models.HashAlgorithm
 }
 
-func (t *GenerateTranscodeTask) Start(wg *sizedwaitgroup.SizedWaitGroup) {
-	defer wg.Done()
-
+func (t *GenerateTranscodeTask) Start() {
 	hasTranscode := HasTranscode(&t.Scene, t.fileNamingAlgorithm)
 	if !t.Overwrite && hasTranscode {
 		return