From c344004e53848cfaac7f2c207c23d54b0ab316c3 Mon Sep 17 00:00:00 2001 From: BenV <165034+benv666@users.noreply.github.com> Date: Sat, 10 Oct 2026 09:46:57 +0200 Subject: [PATCH] fix(actions): settle cancelled reusable workflow runs stuck in cancelling (#39706) Fixes #39702 **Root Cause:** Cancelling a reusable workflow caller wrote it `cancelled` right away, even while a child was still `cancelling`. When the runner acknowledged, the caller did not change, so the run was never refreshed and stayed `cancelling`. SQLite hid this because the child is cancelled a second time, which re-aggregates the caller, while MySQL reports no affected rows for that unchanged update. **Changes:** - A caller now takes its status only from its children: it stays `cancelling` until its last child finishes, and its final status then refreshes the run. - Cancelling a run whose jobs are all done only settles the run, which did not wake the runs waiting for its concurrency group. It now does, so force-cancelling an already stuck run also frees them. --------- Signed-off-by: BenV <165034+benv666@users.noreply.github.com> Co-authored-by: Zettat123 --- models/actions/run_job.go | 31 +++++++++----- models/actions/run_job_test.go | 70 ++++++++++++++++++++++++++++++ services/actions/cancel.go | 7 +++ services/actions/cancel_test.go | 76 +++++++++++++++++++++++++++++++++ 4 files changed, 173 insertions(+), 11 deletions(-) create mode 100644 services/actions/cancel_test.go diff --git a/models/actions/run_job.go b/models/actions/run_job.go index 23c5333b1d9..e8a83a04e27 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -853,25 +853,34 @@ func cancelReusableCaller(ctx context.Context, caller *ActionRunJob, force bool) } // Cancel descendants deepest-first, then the caller: a caller's status is aggregated from its children, - // so each child must reach its final state before its parent caller is re-aggregated. + // so each child must be cancelled before its parent caller is re-read. // A child's ID always exceeds its parent's, so descending ID is a valid deepest-first order. descendants := CollectAllDescendantJobs(caller, attemptJobs) slices.SortFunc(descendants, func(a, b *ActionRunJob) int { return cmp.Compare(b.ID, a.ID) }) + callersWithChildren := make(container.Set[int64]) + for _, d := range descendants { + callersWithChildren.Add(d.ParentJobID) + } - for _, c := range descendants { - cancelled, err := cancelOneJob(ctx, c, force) + for _, job := range append(descendants, caller) { + if !callersWithChildren.Contains(job.ID) { + cancelled, err := cancelOneJob(ctx, job, force) + if err != nil { + return cancelledJobs, err + } + if cancelled != nil { + cancelledJobs = append(cancelledJobs, cancelled) + } + continue + } + // the job is a caller and its children's cascade already re-aggregated it + reloaded, err := GetRunJobByRunAndID(ctx, job.RunID, job.ID) if err != nil { return cancelledJobs, err } - if cancelled != nil { - cancelledJobs = append(cancelledJobs, cancelled) + if reloaded.Status != job.Status { + cancelledJobs = append(cancelledJobs, reloaded) } } - - if c, err := cancelOneJob(ctx, caller, force); err != nil { - return cancelledJobs, err - } else if c != nil { - cancelledJobs = append(cancelledJobs, c) - } return cancelledJobs, nil } diff --git a/models/actions/run_job_test.go b/models/actions/run_job_test.go index 4b428c67079..144ea141381 100644 --- a/models/actions/run_job_test.go +++ b/models/actions/run_job_test.go @@ -7,6 +7,7 @@ import ( "fmt" "testing" + runnerv1 "gitea.dev/actionslib/runner/v1" "gitea.dev/models/db" "gitea.dev/models/unittest" "gitea.dev/modules/timeutil" @@ -397,3 +398,72 @@ func TestForceCancelJobs(t *testing.T) { assert.Equal(t, StatusCancelled, callerAfter.Status) }) } + +func TestCancelJobs_CallerWaitsForCancellingChild(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + ctx := t.Context() + + run := &ActionRun{ + Title: "caller-cancelling-child", + RepoID: 4, + Index: 9811, + OwnerID: 1, + WorkflowID: "test.yaml", + TriggerUserID: 1, + Ref: "refs/heads/master", + CommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0", + Event: "push", + TriggerEvent: "push", + EventPayload: "{}", + Status: StatusRunning, + } + require.NoError(t, db.Insert(ctx, run)) + attempt := &ActionRunAttempt{RepoID: run.RepoID, RunID: run.ID, Attempt: 1, TriggerUserID: 1, Status: StatusRunning} + require.NoError(t, db.Insert(ctx, attempt)) + run.LatestAttemptID = attempt.ID + require.NoError(t, UpdateRun(ctx, run, "latest_attempt_id")) + + newJob := func(name string, parentID int64, isCaller bool) *ActionRunJob { + job := &ActionRunJob{ + RunID: run.ID, + RunAttemptID: attempt.ID, + RepoID: run.RepoID, + OwnerID: run.OwnerID, + CommitSHA: run.CommitSHA, + Name: name, + JobID: name, + Attempt: 1, + Status: StatusRunning, + ParentJobID: parentID, + IsReusableCaller: isCaller, + IsExpanded: isCaller, + } + require.NoError(t, db.Insert(ctx, job)) + return job + } + outer := newJob("outer", 0, true) + inner := newJob("inner", outer.ID, true) + child := newJob("child", inner.ID, false) + + runner := &ActionRunner{UUID: "caller-cancelling-child", Name: "caller-cancelling-child", HasCancellingSupport: true} + require.NoError(t, db.Insert(ctx, runner)) + task := &ActionTask{JobID: child.ID, Attempt: 1, RunnerID: runner.ID, Status: StatusRunning, Started: timeutil.TimeStampNow(), RepoID: run.RepoID, OwnerID: run.OwnerID, CommitSHA: run.CommitSHA} + require.NoError(t, db.Insert(ctx, task)) + child.TaskID = task.ID + _, err := UpdateRunJob(ctx, child, nil, "task_id") + require.NoError(t, err) + + cancelled, err := CancelJobs(ctx, []*ActionRunJob{outer}, false) + require.NoError(t, err) + assert.Len(t, cancelled, 3) + for _, job := range []*ActionRunJob{outer, inner, child} { + assert.Equal(t, StatusCancelling, unittest.AssertExistsAndLoadBean(t, &ActionRunJob{ID: job.ID}).Status, job.Name) + } + + _, err = UpdateTaskByState(ctx, runner.ID, &runnerv1.TaskState{Id: task.ID, Result: runnerv1.Result_RESULT_CANCELLED}) + require.NoError(t, err) + for _, job := range []*ActionRunJob{outer, inner, child} { + assert.Equal(t, StatusCancelled, unittest.AssertExistsAndLoadBean(t, &ActionRunJob{ID: job.ID}).Status, job.Name) + } + assert.Equal(t, StatusCancelled, unittest.AssertExistsAndLoadBean(t, &ActionRun{ID: run.ID}).Status) +} diff --git a/services/actions/cancel.go b/services/actions/cancel.go index 83b56bd392c..a62fb0edd97 100644 --- a/services/actions/cancel.go +++ b/services/actions/cancel.go @@ -11,6 +11,7 @@ import ( actions_model "gitea.dev/models/actions" "gitea.dev/models/db" "gitea.dev/modules/actions/jobparser" + "gitea.dev/modules/log" ) // CancelRun cancels a run's cancellable jobs and returns the run's post-cancellation state. @@ -52,6 +53,12 @@ func cancelRun(ctx context.Context, run *actions_model.ActionRun, jobs []*action if len(updatedJobs) > 0 || reloaded.Status != run.Status { NotifyWorkflowRunStatusUpdate(ctx, reloaded) } + if len(updatedJobs) == 0 && reloaded.Status != run.Status { + // the run's status was updated, so emit it for the emitter to check + if err := EmitJobsIfReadyByRun(run.ID); err != nil { + log.Error("Check jobs of run %d: %v", run.ID, err) + } + } return reloaded, nil } diff --git a/services/actions/cancel_test.go b/services/actions/cancel_test.go new file mode 100644 index 00000000000..ba08b5c1526 --- /dev/null +++ b/services/actions/cancel_test.go @@ -0,0 +1,76 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package actions + +import ( + "testing" + + actions_model "gitea.dev/models/actions" + "gitea.dev/models/db" + "gitea.dev/models/unittest" + "gitea.dev/modules/test" + "gitea.dev/modules/timeutil" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestForceCancelRun_SettledRunWakesConcurrencyWaiters(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + ctx := t.Context() + + // a run left cancelling although its caller and child are all cancelled already + run := &actions_model.ActionRun{ + Title: "settled-run", + RepoID: 4, + Index: 9821, + OwnerID: 1, + WorkflowID: "test.yaml", + TriggerUserID: 1, + Ref: "refs/heads/master", + CommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0", + Event: "push", + TriggerEvent: "push", + EventPayload: "{}", + Status: actions_model.StatusCancelling, + } + require.NoError(t, db.Insert(ctx, run)) + attempt := &actions_model.ActionRunAttempt{RepoID: run.RepoID, RunID: run.ID, Attempt: 1, TriggerUserID: 1, Status: actions_model.StatusCancelling} + require.NoError(t, db.Insert(ctx, attempt)) + run.LatestAttemptID = attempt.ID + require.NoError(t, actions_model.UpdateRun(ctx, run, "latest_attempt_id")) + + newJob := func(name string, parentID int64, isCaller bool) *actions_model.ActionRunJob { + job := &actions_model.ActionRunJob{ + RunID: run.ID, + RunAttemptID: attempt.ID, + RepoID: run.RepoID, + OwnerID: run.OwnerID, + CommitSHA: run.CommitSHA, + Name: name, + JobID: name, + Attempt: 1, + Status: actions_model.StatusCancelled, + Stopped: timeutil.TimeStampNow(), + ParentJobID: parentID, + IsReusableCaller: isCaller, + IsExpanded: isCaller, + } + require.NoError(t, db.Insert(ctx, job)) + return job + } + caller := newJob("caller", 0, true) + child := newJob("child", caller.ID, false) + + var emitted []int64 + defer test.MockVariableValue(&EmitJobsIfReadyByRun, func(runID int64) error { + emitted = append(emitted, runID) + return nil + })() + + got, err := ForceCancelRun(ctx, run, []*actions_model.ActionRunJob{caller, child}) + require.NoError(t, err) + assert.Equal(t, actions_model.StatusCancelled, got.Status) + assert.Equal(t, []int64{run.ID}, emitted) +}