fix(actions): deadlock between PrepareRunAndInsert and UpdateTaskByState

[Zettat123]

Fix #36234

Bug

Logs show PrepareRunAndInsert: InsertRun: Error 1213: Deadlock found, which handleWorkflows silently swallows via log.Error + continue, so the triggered run is dropped.

Root cause

The path UpdateRun -> UpdateRepoRunsNumbers runs the following SQL inside every status-changing transaction:

UPDATE repository
SET num_action_runs        = (SELECT count(*) FROM action_run WHERE repo_id = N),
    num_closed_action_runs = (SELECT count(*) FROM action_run WHERE repo_id = N AND status IN (...))
WHERE id = N;

On any DB that treats subqueries inside an UPDATE as locking reads, this statement takes locks in two steps:

1. The outer UPDATE acquires an X lock on repository[id=N]
2. The embedded SELECT subqueries are evaluated as locking reads, taking S locks on every action_run row matching repo_id = N

Two such concurrent transactions form a cycle via repository[N]:

Tx	Holds	Wants	Blocked by
A: PrepareRunAndInsert (push trigger)	X on inserted action_run row R_A; X on repository[N] (outer UPDATE already through step 1)	S on action_run rows for repo N (subquery, step 2)	B's X lock on R_B
B: UpdateTaskByState (runner callback)	X on action_run row R_B (from UpdateRun)	X on repository[N] (outer UPDATE, step 1)	A's X lock on repository[N]
Cycle	A waits for R_B; B waits for repository[N]		deadlock error -> handleWorkflows swallows -> run lost

PostgreSQL's MVCC reads do not take these locks and SQLite serializes writers, so the symptom only surfaces on MySQL/MSSQL.

Fix

Split UpdateRepoRunsNumbers into small SQLs to avoid locking reads and move it out of DB transactions.

[Zettat123]

Test failure due to the deadlock:

MySQL:
https://github.com/go-gitea/gitea/actions/runs/25822443469/job/75867108596?pr=37692#step:10:97

MSSQL:
https://github.com/go-gitea/gitea/actions/runs/25822443469/job/75867108876?pr=37692#step:10:97

[wxiaoguang]

I don't think the test is really related or should be kept.

[wxiaoguang]

I think you can simply move the "update number" SQL out of the "UpdateTaskByState" transaction.

And split it into small SQLs (two single SELECTs + one single UPDATE)

No need to introduce the cron

[wxiaoguang]

It also needs to move "UpdateRepoRunsNumbers" out of the transaction

[Zettat123]

I think you can simply move the "update number" SQL out of the "UpdateTaskByState" transaction.

And split it into small SQLs (two single SELECTs + one single UPDATE)

No need to introduce the cron

Addressed

I don't think the test is really related or should be kept.

This test was for reproducing the deadlock bug, but I think it’s worth keeping to detect cases where UpdateRepoRunsNumbers is mistakenly placed inside a transaction.

[wxiaoguang]

I don't think the test is really related or should be kept.

This test was for reproducing the deadlock bug, but I think it’s worth keeping to detect cases where UpdateRepoRunsNumbers is mistakenly placed inside a transaction.

if InTransaction {
  setting.PanicInDevOrTesting(...)
}

[Zettat123]

I don't think the test is really related or should be kept.

This test was for reproducing the deadlock bug, but I think it’s worth keeping to detect cases where UpdateRepoRunsNumbers is mistakenly placed inside a transaction.

if InTransaction {
  setting.PanicInDevOrTesting(...)
}

Addressed by 7106acd

[bircni]

@wxiaoguang are you also fine with this?

[wxiaoguang]

@wxiaoguang are you also fine with this?

Yes, it looks good to me.

[GiteaBot]

I was unable to create a backport for 1.26. @Zettat123, please send one manually. 🍵

go run ./contrib/backport 37692
...  // fix git conflicts if any
go run ./contrib/backport --continue

[Zettat123]

I was unable to create a backport for 1.26. @Zettat123, please send one manually. 🍵

go run ./contrib/backport 37692
...  // fix git conflicts if any
go run ./contrib/backport --continue

#37718