Skip to content

Misc builder fixes#14971

Merged
Ericson2314 merged 2 commits intoNixOS:masterfrom
obsidiansystems:misc-builder-fixes
Jan 11, 2026
Merged

Misc builder fixes#14971
Ericson2314 merged 2 commits intoNixOS:masterfrom
obsidiansystems:misc-builder-fixes

Conversation

@Ericson2314
Copy link
Member

Motivation

Two fixes to the worker/goal machinery. See each commit for details.

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

We have to call into both branches no matter what.
When a goal with an active child process is destroyed (e.g., during
failure cascades without `--keep-going`), the child process gets killed
but `childTerminated` was never called. This left stale entries in the
worker's `children` list.

Fix this by ensuring `childTerminated` is called from destructors:

- `DerivationBuilderImpl::killChild` now calls `childTerminated` via
  the `miscMethods` callback.

- `HookInstance` gains an `onKillChild` callback that is invoked from
  its destructor when killing the process. `buildWithHook` sets this
  callback to call `childTerminated`.

To make these calls safe from destructors (where the goal object may be
partially destroyed), add a new overload of `Worker::childTerminated`
that takes an explicit `JobCategory` parameter instead of calling the
virtual method `Goal::jobCategory`. The original overload still exists
for convenience for normal (non-destructor) call sites.
@Ericson2314 Ericson2314 requested a review from xokdvium January 11, 2026 04:55
Copy link
Contributor

@xokdvium xokdvium left a comment

Choose a reason for hiding this comment

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

We should probably assert in some places that the goal pointer is not null.

@Ericson2314 Ericson2314 added this pull request to the merge queue Jan 11, 2026
Merged via the queue into NixOS:master with commit 252aff5 Jan 11, 2026
14 checks passed
@Ericson2314 Ericson2314 deleted the misc-builder-fixes branch January 11, 2026 21:47
JustAGuyTryingHisBest pushed a commit to JustAGuyTryingHisBest/nix that referenced this pull request Jan 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants