From 5157db5a6a92617e1232845fbbc3c67fcdcaf5d5 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Wed, 6 May 2026 12:17:36 +0300 Subject: Move background goroutine start out of wireDeps into startBackgroundWorkers (task z0) --- cmd/player/main.go | 41 ++++++++++++--------- cmd/player/main_test.go | 97 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 17 deletions(-) (limited to 'cmd') diff --git a/cmd/player/main.go b/cmd/player/main.go index 0a7bacb..0e96e5f 100644 --- a/cmd/player/main.go +++ b/cmd/player/main.go @@ -48,6 +48,7 @@ type appDeps struct { scanner scanner.Scanner gcWorker *service.GCWorker logger *slog.Logger + appCtx context.Context } // parseVersionFlag parses CLI flags and returns whether --version was requested. @@ -98,23 +99,6 @@ func wireDeps(cfg *internal.Config, store repository.Store, logger *slog.Logger, podcastSvc := service.NewPodcastService(store, clk, cfg.MediaRoot, helper, prober, thumbGen, cfg.PodcastCheckMinutes) gcWorker := service.NewGCWorker(store, clk, cfg.MediaRoot, time.Duration(cfg.GCIntervalMinutes)*time.Minute, logger) - gcWorker.Start() - - // Start podcast feed background checker. - go func() { - ticker := time.NewTicker(time.Duration(cfg.PodcastCheckMinutes) * time.Minute) - defer ticker.Stop() - for { - select { - case <-ticker.C: - if err := podcastSvc.CheckFeeds(context.Background()); err != nil { - logger.Error("podcast feed check failed", "err", err) - } - case <-appCtx.Done(): - return - } - } - }() return &appDeps{ store: store, @@ -130,9 +114,31 @@ func wireDeps(cfg *internal.Config, store repository.Store, logger *slog.Logger, scanner: fsScanner, gcWorker: gcWorker, logger: logger, + appCtx: appCtx, } } +// startBackgroundWorkers launches background goroutines (GC, podcast feed checker). +func startBackgroundWorkers(deps *appDeps) { + deps.gcWorker.Start() + + // Start podcast feed background checker. + go func() { + ticker := time.NewTicker(time.Duration(deps.cfg.PodcastCheckMinutes) * time.Minute) + defer ticker.Stop() + for { + select { + case <-ticker.C: + if err := deps.podcastSvc.CheckFeeds(context.Background()); err != nil { + deps.logger.Error("podcast feed check failed", "err", err) + } + case <-deps.appCtx.Done(): + return + } + } + }() +} + // ensureSignalChannel returns the provided channel or creates a new one wired // to OS interrupt signals. func ensureSignalChannel(sigCh <-chan os.Signal) <-chan os.Signal { @@ -214,6 +220,7 @@ func runWithSignal(args []string, sigCh <-chan os.Signal) error { deps := wireDeps(cfg, store, logger, appCtx) defer deps.gcWorker.Stop() + startBackgroundWorkers(deps) staticFS := http.Dir("web") remuxer := probe.NewFFRemuxer() diff --git a/cmd/player/main_test.go b/cmd/player/main_test.go index 29d36f0..907e320 100644 --- a/cmd/player/main_test.go +++ b/cmd/player/main_test.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "context" "io" "log/slog" "os" @@ -185,3 +186,99 @@ func TestRunWithSignal_ServerErrorPath(t *testing.T) { t.Fatal("expected error when server cannot bind privileged port") } } + +func TestWireDeps_DoesNotStartBackgroundWorkers(t *testing.T) { + // wireDeps must only construct dependencies; it must not start any background goroutines. + tmpDir := t.TempDir() + dbPath := filepath.Join(tmpDir, "test.db") + mediaRoot := filepath.Join(tmpDir, "media") + if err := os.MkdirAll(mediaRoot, 0o755); err != nil { + t.Fatalf("mkdir media root: %v", err) + } + + store, err := repository.Open(dbPath) + if err != nil { + t.Fatalf("open db: %v", err) + } + defer func() { + if err := store.Close(); err != nil { + t.Logf("close db: %v", err) + } + }() + + cfg := &internal.Config{ + SessionTimeoutHours: 1, + GCIntervalMinutes: 1, + PodcastCheckMinutes: 1, + MediaRoot: mediaRoot, + } + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + deps := wireDeps(cfg, store, logger, ctx) + if deps.gcWorker == nil { + t.Fatal("expected gcWorker to be non-nil") + } + + // We call Stop() immediately. If Start() had been called this is safe (idempotent). + // If Start() was NOT called, the internal stopCh is still open, so Stop() must handle it gracefully. + deps.gcWorker.Stop() +} + +func TestStartBackgroundWorkers_StartsAndStops(t *testing.T) { + // startBackgroundWorkers should launch background goroutines that exit + // cleanly when the app context is cancelled. + tmpDir := t.TempDir() + dbPath := filepath.Join(tmpDir, "test.db") + mediaRoot := filepath.Join(tmpDir, "media") + if err := os.MkdirAll(mediaRoot, 0o755); err != nil { + t.Fatalf("mkdir media root: %v", err) + } + + store, err := repository.Open(dbPath) + if err != nil { + t.Fatalf("open db: %v", err) + } + defer func() { + if err := store.Close(); err != nil { + t.Logf("close db: %v", err) + } + }() + + cfg := &internal.Config{ + SessionTimeoutHours: 1, + GCIntervalMinutes: 1, + PodcastCheckMinutes: 1, + MediaRoot: mediaRoot, + } + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + deps := wireDeps(cfg, store, logger, ctx) + startBackgroundWorkers(deps) + + // Give the goroutines a moment to start. + time.Sleep(50 * time.Millisecond) + + // Cancel the app context; workers should exit. + cancel() + + // Stop the GC worker explicitly (safe and idempotent). + deps.gcWorker.Stop() + + // No explicit assertion for goroutine exit beyond the fact that we have not leaked; + // the final goroutine dump check in the test run will catch leaks. +} + +func TestStartBackgroundWorkers_NilDepsPanics(t *testing.T) { + // Verify defensive behaviour: passing a nil pointer should panic quickly + // so the bug is surfaced at start-up rather than later as a nil dereference. + defer func() { + if r := recover(); r == nil { + t.Fatal("expected panic when startBackgroundWorkers receives nil") + } + }() + startBackgroundWorkers(nil) +} -- cgit v1.2.3