Fix scheduler goroutine WaitGroup leak causing shutdown deadlock

Critical bug fix: The scheduler's runSettingsChecker() goroutine was
started but never marked as complete in the WaitGroup, causing
scheduler.Stop() to hang indefinitely waiting for wg.Wait().

Changes:
- Add defer s.wg.Done() call in scheduler.Start() goroutine wrapper
- Update scheduler tests to properly call worker.Shutdown()
- Add nil check for timer.Stop() to prevent panics from nil timers
- Fix TestScheduler_StopWithActiveTimers to use proper shutdown sequence

Impact:
- Fixes test hanging issue in `make test` command
- Enables graceful shutdown of scheduler in production
- Prevents goroutine leaks in long-running applications
- All unit tests now complete successfully

Root cause: WaitGroup.Add(1) was called but Done() was never called,
creating an imbalance that caused wg.Wait() to block forever.
This commit is contained in:
2026-02-09 10:13:48 -05:00
parent 11070fbf25
commit 70acecc33a
2 changed files with 17 additions and 3 deletions
+7 -2
View File
@@ -66,7 +66,10 @@ func NewScheduler(worker *Worker, db Database) *Scheduler {
func (s *Scheduler) Start() {
s.wg.Add(1)
go s.runSettingsChecker()
go func() {
defer s.wg.Done()
s.runSettingsChecker()
}()
}
func (s *Scheduler) Stop() {
@@ -74,7 +77,9 @@ func (s *Scheduler) Stop() {
s.mu.Lock()
for _, timer := range s.timers {
timer.Stop()
if timer != nil {
timer.Stop()
}
}
s.timers = make(map[string]*time.Timer)
s.mu.Unlock()
+10 -1
View File
@@ -9,6 +9,7 @@ import (
func TestScheduler_NewScheduler(t *testing.T) {
worker := NewWorker(1)
defer worker.Shutdown()
// Use nil database interface for basic testing
scheduler := NewScheduler(worker, nil)
@@ -31,10 +32,12 @@ func TestScheduler_StartStop(t *testing.T) {
// Stop should not panic
scheduler.Stop()
worker.Shutdown()
}
func TestScheduler_UpdateScanSettings(t *testing.T) {
worker := NewWorker(1)
defer worker.Shutdown()
scheduler := NewScheduler(worker, nil)
userID := "test-user-123"
@@ -54,6 +57,7 @@ func TestScheduler_UpdateScanSettings(t *testing.T) {
func TestScheduler_UpdateScanSettings_Disabled(t *testing.T) {
worker := NewWorker(1)
defer worker.Shutdown()
scheduler := NewScheduler(worker, nil)
userID := "test-user-456"
@@ -72,6 +76,7 @@ func TestScheduler_UpdateScanSettings_Disabled(t *testing.T) {
func TestScheduler_UpdateScanSettings_Overwrite(t *testing.T) {
worker := NewWorker(1)
defer worker.Shutdown()
scheduler := NewScheduler(worker, nil)
userID := "test-user-789"
@@ -110,13 +115,14 @@ func TestScheduler_StopWithActiveTimers(t *testing.T) {
scheduler.mu.Unlock()
assert.Equal(t, 0, timerCount)
worker.Shutdown()
}
func TestScheduler_ConcurrentAccess(t *testing.T) {
worker := NewWorker(1)
scheduler := NewScheduler(worker, nil)
scheduler.Start()
defer scheduler.Stop()
// Concurrent updates should not cause race conditions
done := make(chan bool, 10)
@@ -140,4 +146,7 @@ func TestScheduler_ConcurrentAccess(t *testing.T) {
scheduler.mu.Unlock()
assert.Equal(t, 10, settingCount)
scheduler.Stop()
worker.Shutdown()
}