Skip to content

daemon: keep SIGCHLD pipe alive until process exit - #16517

Merged
xokdvium merged 1 commit into
NixOS:masterfrom
booxter:abort-on-sigchld
Sep 25, 2026
Merged

xokdvium merged 1 commit into
NixOS:masterfrom
booxter:abort-on-sigchld

Conversation

@booxter

@booxter booxter commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Motivation

The daemon can enter static destruction while a worker is still exiting. Give
the self-pipe process lifetime to serve any SIGCHLD during shutdown.

Context

The crash report showed this path:

    exit
    __cxa_finalize_ranges
    sigtramp
    nix::sigChldHandler
    abort

I was trying to reproduce a different issue (nix-daemon seemingly not restarted after nix-darwin activation script supposedly sent SIGHUP to it). I was not able to hit my failure, but I did hit a SIGABRT in the daemon instead.

Reproduction script

Generated with Codex (gpt-5.6-sol high).

#!/usr/bin/env python3

import os
import signal
import socket
import subprocess
import sys
import tempfile
import time


nix = sys.argv[1] if len(sys.argv) > 1 else "nix"

# The bug is a shutdown race, so retry with a fresh disposable daemon.
for attempt in range(1, 101):
    with tempfile.TemporaryDirectory(prefix="nix-daemon-sigchld-") as directory:
        socket_path = directory + "/daemon.sock"
        environment = os.environ | {
            "NIX_CONF_DIR": directory,
            "NIX_USER_CONF_FILES": "/dev/null",
            "NIX_CONFIG": "",
        }
        process = subprocess.Popen(
            [
                nix,
                "--extra-experimental-features",
                "nix-command",
                "--store",
                "dummy://",
                "daemon",
                "--socket-path",
                socket_path,
            ],
            stdout=subprocess.DEVNULL,
            stderr=subprocess.DEVNULL,
            env=environment,
        )

        # Hold raw connections open so forked workers block waiting for their
        # protocol handshake.
        clients = []
        while len(clients) < 8:
            client = socket.socket(socket.AF_UNIX)
            try:
                client.connect(socket_path)
                clients.append(client)
            except OSError:
                client.close()
                if process.poll() is not None:
                    raise SystemExit(f"daemon exited early: {process.returncode}")
                time.sleep(0.001)
        time.sleep(0.01)

        # Start daemon shutdown, then release its workers. Their SIGCHLD can
        # race destruction of the handler's self-pipe and trigger abort().
        process.send_signal(signal.SIGHUP)
        for client in clients:
            client.close()
        process.wait()

        if process.returncode == -signal.SIGABRT:
            print(f"reproduced SIGABRT after {attempt} attempts")
            raise SystemExit(0)
        if process.returncode != 0:
            raise SystemExit(f"unexpected daemon exit: {process.returncode}")

raise SystemExit("SIGABRT not reproduced after 100 attempts")

Add 👍 to pull requests you find important.

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

The daemon can enter static destruction while a worker is still exiting.
Give the self-pipe process lifetime to serve any SIGCHLD during
shutdown.

The crash report showed this path:

    exit
    __cxa_finalize_ranges
    sigtramp
    nix::sigChldHandler
    abort
@booxter
booxter requested a review from edolstra as a code owner September 25, 2026 00:43
@github-actions github-actions Bot added the new-cli Relating to the "nix" command label Sep 25, 2026
@xokdvium

Copy link
Copy Markdown
Contributor

Eh, we really need to disable all global destructors...

@xokdvium
xokdvium added this pull request to the merge queue Sep 25, 2026
Merged via the queue into NixOS:master with commit 1805795 Sep 25, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-cli Relating to the "nix" command

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants