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

@@ -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
+		task.Start()
-		wg.Add(1)
-		task.Start(&wg)
 
 		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()

pkg/manager/task.go

@@ -1,8 +1,6 @@
 package manager
 
-import "sync"
-
 type Task interface {
-	Start(wg *sync.WaitGroup)
+	Start()
 	GetDescription() string
 }

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) {
+func (t *GenerateMarkersTask) Start() {
-	defer wg.Done()
-
 	if t.Scene != nil {
 		t.generateSceneMarkers()
 	}

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) {
+func (t *GeneratePhashTask) Start() {
-	defer wg.Done()
-
 	if !t.shouldGenerate() {
 		return
 	}

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) {
+func (t *GeneratePreviewTask) Start() {
-	defer wg.Done()
-
 	videoFilename := t.videoFilename()
 	videoChecksum := t.Scene.GetHash(t.fileNamingAlgorithm)
 	imageFilename := t.imageFilename()

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) {
+func (t *GenerateScreenshotTask) Start() {
-	defer wg.Done()
-
 	scenePath := t.Scene.Path
 	probeResult, err := ffmpeg.NewVideoFile(instance.FFProbePath, scenePath, false)
 

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) {
+func (t *GenerateSpriteTask) Start() {
-	defer wg.Done()
-
 	if !t.Overwrite && !t.required() {
 		return
 	}

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) {
+func (t *ImportTask) Start() {
-	defer wg.Done()
-
 	if t.TmpZip != "" {
 		defer func() {
 			err := utils.RemoveDir(t.BaseDir)

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) {
+func (t *MigrateHashTask) Start() {
-	defer wg.Done()
-
 	if !t.Scene.OSHash.Valid || !t.Scene.Checksum.Valid {
 		// nothing to do
 		return

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) {
+func (t *ScanTask) Start() {
-	defer wg.Done()
-
 	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)
+		subTask.Start()
-		iwg.Add()
-		subTask.Start(&iwg)
 		return nil
 	})
 	if err != nil {

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) {
+func (t *StashBoxPerformerTagTask) Start() {
-	defer wg.Done()
-
 	t.stashBoxPerformerTag()
 }
 

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) {
+func (t *GenerateTranscodeTask) Start() {
-	defer wg.Done()
-
 	hasTranscode := HasTranscode(&t.Scene, t.fileNamingAlgorithm)
 	if !t.Overwrite && hasTranscode {
 		return