From 3ec20dfb55451dedc44f8c4bcf91f67e7e4ccb81 Mon Sep 17 00:00:00 2001 From: Goon Date: Fri, 12 Jun 2026 22:44:45 +0700 Subject: [PATCH] fix(cron): wait for in-flight jobs on shutdown --- docs/project-changelog.md | 12 ++++++++ internal/cron/service.go | 2 ++ internal/cron/service_execution.go | 2 ++ internal/cron/service_test.go | 49 ++++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+) diff --git a/docs/project-changelog.md b/docs/project-changelog.md index d81e98b2..c82af4f1 100644 --- a/docs/project-changelog.md +++ b/docs/project-changelog.md @@ -6,6 +6,18 @@ Significant changes, features, and fixes in reverse chronological order. ## 2026-06-12 +### Cron scheduler shutdown drain (post-merge CI follow-up) + +**Fixes** + +- Made cron service shutdown wait for scheduler-launched job executions to + finish so in-flight status persistence cannot outlive `Stop()`. + +**Tests** + +- Added regression coverage proving `Stop()` waits for a blocked in-flight job + before returning. + ### Bundled GoClaw gateway administration skill (issue #175) **Changes** diff --git a/internal/cron/service.go b/internal/cron/service.go index 4fd8f37f..9f3b09c9 100644 --- a/internal/cron/service.go +++ b/internal/cron/service.go @@ -14,6 +14,7 @@ type Service struct { running bool stopChan chan struct{} loopWG sync.WaitGroup + jobWG sync.WaitGroup mu sync.Mutex runLog []RunLogEntry // in-memory run history (last 200 entries) retryCfg RetryConfig // retry config for failed jobs @@ -114,6 +115,7 @@ func (cs *Service) Stop() { cs.mu.Unlock() cs.loopWG.Wait() + cs.jobWG.Wait() slog.Info("cron service stopped") } diff --git a/internal/cron/service_execution.go b/internal/cron/service_execution.go index fd8a7a5d..91d08079 100644 --- a/internal/cron/service_execution.go +++ b/internal/cron/service_execution.go @@ -209,6 +209,7 @@ func (cs *Service) checkJobs() { } } cs.saveUnsafe() + cs.jobWG.Add(len(dueJobs)) cs.mu.Unlock() // Execute jobs in parallel without blocking the runLoop. @@ -217,6 +218,7 @@ func (cs *Service) checkJobs() { // due jobs. Now each job runs independently with panic recovery. for _, dj := range dueJobs { go func(id string, scheduledAtMS int64) { + defer cs.jobWG.Done() defer safego.Recover(nil, "job_id", id) cs.executeJobByID(id, scheduledAtMS) }(dj.id, dj.scheduledAtMS) diff --git a/internal/cron/service_test.go b/internal/cron/service_test.go index 15ec1672..b1697029 100644 --- a/internal/cron/service_test.go +++ b/internal/cron/service_test.go @@ -306,6 +306,55 @@ func TestService_JobFailure_Updates_LastError(t *testing.T) { } } +func TestService_Stop_WaitsForInFlightJob(t *testing.T) { + setFastTick(t) + dir := t.TempDir() + storePath := filepath.Join(dir, "cron.json") + + entered := make(chan struct{}) + release := make(chan struct{}) + handler := func(job *Job) (string, error) { + close(entered) + <-release + return "done", nil + } + + cs := NewService(storePath, handler) + interval := int64(10) + if _, err := cs.AddJob("blocking", Schedule{Kind: "every", EveryMS: &interval}, "block", false, "", "", ""); err != nil { + t.Fatalf("AddJob error: %v", err) + } + if err := cs.Start(); err != nil { + t.Fatalf("Start error: %v", err) + } + + select { + case <-entered: + case <-time.After(time.Second): + cs.Stop() + t.Fatal("job handler did not start") + } + + stopped := make(chan struct{}) + go func() { + cs.Stop() + close(stopped) + }() + + select { + case <-stopped: + t.Fatal("Stop returned while job handler was still running") + case <-time.After(50 * time.Millisecond): + } + + close(release) + select { + case <-stopped: + case <-time.After(time.Second): + t.Fatal("Stop did not return after in-flight job completed") + } +} + // --- Persistence: save and reload --- func TestService_Persistence_Roundtrip(t *testing.T) {