kyush/forgejo
1
0
Fork 0
forked from mirrors/forgejo
Code Pull requests Activity

[v14.0/forgejo] fix: don't return AdditionalTasks from FetchTask if there is no Task (#10909)

Browse source 
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/10899

Possible bug that could cause https://code.forgejo.org/forgejo/runner/issues/1302: by picking more tasks after the first `PickTask` didn't find anything, they'll be returned in `AdditionalTasks`.  But the runner doesn't act upon additional tasks if there is no "first" task.

I can't see a practical way to cover this with an automated test other than mutating the production code to provide a synchronization point between the two operations.

## Checklist

The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).

### Tests

- I added test coverage for Go changes...
  - [ ] in their respective `*_test.go` for unit tests.
  - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
  - [ ] in `web_src/js/*.test.js` if it can be unit tested.
  - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).

### Documentation

- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.

### Release notes

- [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.

*The decision if the pull request will be shown in the release notes is up to the mergers / release team.*

The content of the `release-notes/<pull request number>.md` file will serve as the basis for the release notes. If the file does not exist, the title of the pull request will be used instead.

Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10909
Co-authored-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
Co-committed-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
This commit is contained in:
forgejo-backport-action 2026-01-18 02:35:02 +01:00 • committed by Mathieu Fenniak
commit ec28d5885d
1 changed files with 15 additions and 15 deletions
Download patch file Download diff file Expand all files Collapse all files

30
routers/api/actions/runner/runner.go
View file

@ -162,22 +162,22 @@ func (s *Service) FetchTask(
return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("pick task: %w", err))
} else if ok {
task = t
}
taskCapacity := req.Msg.GetTaskCapacity()
taskCapacity-- // remove 1 for the task already fetched as `task`
for taskCapacity > 0 {
if t, ok, err := actions_service.PickTask(ctx, runner); err != nil {
// Don't return an error to the client/runner -- we've already assigned one-or-more tasks to the runner
// and if we don't return them, they can't be picked up by another runner and will become zombie tasks.
// Log the error and return the tasks we've assigned so far.
log.Error("pick task failed: %v", err)
break
} else if ok {
additionalTasks = append(additionalTasks, t)
taskCapacity--
} else {
break
taskCapacity := req.Msg.GetTaskCapacity()
taskCapacity-- // remove 1 for the task already fetched as `task`
for taskCapacity > 0 {
if t, ok, err := actions_service.PickTask(ctx, runner); err != nil {
// Don't return an error to the client/runner -- we've already assigned one-or-more tasks to the runner
// and if we don't return them, they can't be picked up by another runner and will become zombie tasks.
// Log the error and return the tasks we've assigned so far.
log.Error("pick task failed: %v", err)
break
} else if ok {
additionalTasks = append(additionalTasks, t)
taskCapacity--
} else {
break
}
}
}
}