Skip to content

[Backport 2.33-maintenance] Fix destruction of DerivationBuilder implementations#15078

Merged
Ericson2314 merged 1 commit into
NixOS:2.33-maintenancefrom
xokdvium:backport-15072-to-2.33-maintenance
Jan 25, 2026
Merged

[Backport 2.33-maintenance] Fix destruction of DerivationBuilder implementations#15078
Ericson2314 merged 1 commit into
NixOS:2.33-maintenancefrom
xokdvium:backport-15072-to-2.33-maintenance

Conversation

@xokdvium
Copy link
Copy Markdown
Contributor

Motivation

Manual backport of #15072 to 2.33-maintenance.
(cherry picked from commit b752c5c)

Context


Add 👍 to pull requests you find important.

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

This unsures that we call the correct virtual functions when destroying a particular
DerivationBuilder.

Usually the order of destructors is in the reverse order of inheritance:

ChrootLinuxDerivationBuilder -> ChrootDerivationBuilder -> DerivationBuilderImpl

autoDelChroot was being destroyed before the DerivationBuilderImpl::killChild was
run and it would fail to clean up the chroot directory, since there were still processes
writing to it. Note that ChrootLinuxDerivationBuilder::killSandbox was never run in
the interrupted case at all, since virtual functions in destructors do not call derived class
methods.

I could reproduce the issue with the following derivation:

let
  pkgs = import <nixpkgs> { };
in
pkgs.runCommand "chroot-cleanup-race" { } ''
  mkdir -p $out

  for i in $(seq 1 200); do
    (
      mkfifo $out/fifo$i
      cat $out/fifo$i > /dev/null &

      while true; do
        : > $out/file$i
      done
    ) &
  done

  sleep 0.05
  echo done > $out/main
''

While interrupting it manually when it would hang.

Wrapping the unique pointer in a custom deleter function we can run all
of the necessary clean up code consistently and calling the right virtual
functions. Ideally we'd have a lint that bans the usage of virtual functions
in destructors completely.

(cherry picked from commit b752c5c)
@xokdvium xokdvium requested a review from Ericson2314 as a code owner January 24, 2026 23:26
@Ericson2314 Ericson2314 enabled auto-merge January 24, 2026 23:51
@Ericson2314 Ericson2314 merged commit a2b044d into NixOS:2.33-maintenance Jan 25, 2026
15 checks passed
@Ericson2314 Ericson2314 added the backport 2.32-maintenance Automatically creates a PR against the branch label Jan 26, 2026
@internal-nix-ci
Copy link
Copy Markdown

Backport failed for 2.32-maintenance, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.32-maintenance
git worktree add -d .worktree/backport-15078-to-2.32-maintenance origin/2.32-maintenance
cd .worktree/backport-15078-to-2.32-maintenance
git switch --create backport-15078-to-2.32-maintenance
git cherry-pick -x 70ecd8c8a9123bb6e79a19559fc1b5a5310ddbc8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.32-maintenance Automatically creates a PR against the branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants