Skip to content

allocs rpc: fix client panic on nil allocs#28187

Merged
gulducat merged 1 commit into
mainfrom
fix-nil-alloc-panic
Jun 26, 2026
Merged

allocs rpc: fix client panic on nil allocs#28187
gulducat merged 1 commit into
mainfrom
fix-nil-alloc-panic

Conversation

@gulducat

@gulducat gulducat commented Jun 26, 2026

Copy link
Copy Markdown
Member

Description

A client can panic on a nil alloc sent by the server if the alloc gets GC'd at an innoportune moment, namely between two distinct RPC calls in the same loop.

The server should never return a nil alloc. Instead, return a partial slice that is shorter than the number of allocs requested.

Fixes: #28172

Note: At this time, the client does not make use of the partial list; it simply discards the results and retries in a couple seconds (client/client.go line 2683). Potential client optimizations may be revisited as a follow-up.

Testing

Server-side unit tests ensures no nil allocs in the response. I'll explore Client-side tests, but it'll take some extra doing to set up mocks for the RPC calls.

Contributor Checklist

  • Changelog Entry If this PR changes user-facing behavior, please generate and add a
    changelog entry using the make cl command.
  • Testing Please add tests to cover any new functionality or to demonstrate bug fixes and
    ensure regressions will be caught.
  • Documentation If the change impacts user-facing functionality such as the CLI, API, UI,
    and job configuration, please update the Nomad product documentation, which is stored in the
    web-unified-docs repo. Refer to the web-unified-docs contributor guide for docs guidelines.
    Please also consider whether the change requires notes within the upgrade
    guide
    . If you would like help with the docs, tag the nomad-docs team in this PR.
  • LLM Usage If an LLM was used to generate any code, please ensure and confirm you have read
    and followed the AI usage guide.

Reviewer Checklist

  • Backport Labels Please add the correct backport labels as described by the internal
    backporting document.
  • Commit Type Ensure the correct merge method is selected which should be "squash and merge"
    in the majority of situations. The main exceptions are long-lived feature branches or merges where
    history should be preserved.
  • Enterprise PRs If this is an enterprise only PR, please add any required changelog entry
    within the public repository.
  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

A client can panic on a nil alloc sent by the server if the alloc gets
GC'd at an innoportune moment, namely between two distinct RPC calls
in the same loop.

The server should never return a nil alloc. Instead, return a partial
slice that is shorter than the number of allocs requested.

Note: At this time, the client does not make use of the partial list;
it simply discards the results and retries in a couple seconds
(client/client.go line 2683). Potential client optimizations may be
revisited as a follow-up.
@gulducat gulducat requested a review from tgross June 26, 2026 20:10
@gulducat gulducat requested review from a team as code owners June 26, 2026 20:10
@gulducat gulducat added the backport/2.0.x backport to 2.0.x release line label Jun 26, 2026

@tgross tgross left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Note: At this time, the client does not make use of the partial list; it simply discards the results and retries in a couple seconds (client/client.go line 2683). Potential client optimizations may be revisited as a follow-up.

Well-worded 👍

@gulducat gulducat merged commit 67a9aa5 into main Jun 26, 2026
40 checks passed
@gulducat gulducat deleted the fix-nil-alloc-panic branch June 26, 2026 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/2.0.x backport to 2.0.x release line

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nomad client panic in Allocation.Canonicalize after short-lived periodic batch allocation completes

2 participants