Skip to content

libc: a wait under an ignored SIGCHLD answers no child that ended - #1030

Merged
Japabu merged 1 commit into
mainfrom
wt/toyos-proc-waitrace
Oct 22, 2026
Merged

Japabu merged 1 commit into
mainfrom
wt/toyos-proc-waitrace

Conversation

@Japabu

@Japabu Japabu commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

With SIGCHLD ignored, libc's wait_child (sdk/libc/src/spawn.rs) could answer a child as reaped, though POSIX has an ignored SIGCHLD keep no child for a wait and answer ECHILD once none is left. signal_sigchld_ignored caught it once, during #1012's rebase.

The cause is a race between two reads. Each turn of the wait:

  1. ran forget_ended(), which drops every child that has ended, under one lock;
  2. read the child table again, under a second lock, and reaped the first named child it found ended.

A child that ended between the two reads was found by the second, reaped and returned.

Resulting behaviour

Each turn reads every child's end once, under the one lock that decides from that read. spawnreq::turn makes the decision:

  • With SIGCHLD ignored, every child that read saw ended is forgotten and none is answered. The wait answers ECHILD once none it names is left, and otherwise watches the rest.
  • Otherwise the wait answers the first named child that ended, watches the rest, or answers ECHILD when it names none.

A child that ends after the read belongs to the next turn, and its handle's readiness wakes that turn.

forget_ended() shares the same removal (forget). The process track's record now names the new host law.

Checks and evidence

All at 92cc5474e, on origin/main 35e930136:

  • Host laws in sdk/libc/arch/src/spawn_requests.rs: an_ignored_sigchld_answers_no_ended_child and a_wait_takes_the_first_ended_child. They are deterministic: one read's ends go in, and a decision comes out.

  • Mutation (checked patch, built, restored): the ignored arm of turn is skipped, which brings back the old answer, an ended child reaped under SIG_IGN. an_ignored_sigchld_answers_no_ended_child fails, exit 101.

  • cargo run -- --ci host: exit 0, 101 steps green.

  • cargo test --test toyos-build -- libc_children (QEMU x86-64): signal_sigchld_ignored passes. The boot still exits 1, on spawn_children_cap, which also fails on the base:

    • With the fix: the spawn loop's posix_spawn answered ENOMEM (12).
    • With the fix reverted (git revert -n, the same boot, then restored): child 240 ended by SIGSEGV (status 0xb).

    Both are that case's 254 held children running out of guest memory. issues/root-in-memory-costs-its-whole-size.md records that ceiling (refused at 241, ENOMEM). This PR does not change it, because the spawn loop never waits. libc_children is red on main at 35e930136, independently of this change.

The guest race itself is not reproduced on demand. The host laws establish the decision, and the single read is visible in the diff.

Evidence: https://github.com/ToyOSOrg/evidence/tree/97b3afdc21eb72c555c51dcb468920c971d5e24c (waitrace/)

🤖 Generated with Claude Code

https://claude.ai/code/session_01PXEk8BBMDT8tXu4AnENB9Q

wait_child forgot every ended child under one lock, then read the table
again under another and reaped the first named child it found ended. A
child that ended between the two reads was answered as reaped, where
SIGCHLD ignored has it kept for no wait and the wait answer ECHILD once
none is left. signal_sigchld_ignored saw it once.

Each turn now reads every child's end once, under the lock that decides
from it: `spawnreq::turn` makes the decision from that one read, and with
SIGCHLD ignored it forgets each ended child and answers none. A child
that ends after the read is the next turn's, which the poller's wake on
its handle brings.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PXEk8BBMDT8tXu4AnENB9Q
@Japabu

Japabu commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

Review of #1030, round 1 (head 92cc547, base origin/main 35e9301)

Architecture

The fix sits where the defect was: libc's own child table in sdk/libc/src/spawn.rs. The kernel, the ABI and other protocols are unchanged. The root cause is described correctly. A child's end is not serialised by CHILDREN's lock, so two try_wait reads can disagree even under one lock. Merging the two critical sections would not have been enough; what fixes it is a single read per child that drives both the forget and the answer, and that is what the diff does. forget_ended and the wait now share forget. turn is a small pure decision placed in spawnreq beside plan and wait_status, which are already host-tested, so it adds no new boundary. Net change: +87/-29 in total, of which about +36 is production code and +19 is tests.

Checks requested

  • Every wait variant goes through the single read. waitpid and wait call wait4, and wait4 calls wait_child(pid, !WNOHANG). pid > 0, -1, 0 (own group) and < -1 (group -pid) all select through names on the same ended snapshot. WNOHANG and blocking differ only in the Watch arm. spawn_child and act still call forget_ended, which now reads each child once as well. No other path reads the table to answer a wait.
  • POSIX SIG_IGN. Under an ignored SIGCHLD, turn never returns Take, every child the read saw ended is forgotten (including ones the pid does not name), the wait watches the running named children and answers ECHILD once none is left. With WNOHANG it answers 0 while a named child runs and ECHILD otherwise. This matches POSIX for wait/waitpid(-1). One divergence is pre-existing and not introduced here: for waitpid(p), POSIX says the caller "shall block until all of the children ... terminate", but libc waits only for the named ones. Linux behaves the same way, and the PR description describes this behaviour accurately. SA_NOCLDWAIT is not defined in sdk/libc/include/signal.h, also pre-existing and outside this PR.
  • Lost wakeups. Handle copies are made under the lock from the same snapshot. A child that ends after the read is still in running. kernel/src/object/process.rs says that an OP_WATCH reads the end fact when it is submitted, so a copy whose child has already ended completes at once. A child started after the read is covered by children_seen() and the wake pipe, as before. The structure here is unchanged from main.
  • Locks. children_ignored() takes and releases STATE before CHILDREN is taken. Nothing under CHILDREN takes STATE (reap calls children_changed only after dropping CHILDREN). No new nesting. If the disposition changes between the ignored read and the lock, the turn is decided under the disposition in force when the call began, which is acceptable.
  • Laws and control. The two host laws pin turn's table, including the empty sets. Evidence waitrace/mut-ignored-answers-an-ended-child.{patch,log} (if false && ignored) shows an_ignored_sigchld_answers_no_ended_child failing at spawn_requests.rs:191, exit 101, at head 92cc547. The control shows that the law detects the defective decision. It cannot show the single read, because reverting the spawn.rs change leaves turn untouched. The PR says this plainly ("the single read is visible in the diff"). I confirm it by inspection: there is one try_wait per child per turn, and reap's re-read only re-confirms an end, which is permanent.
  • Independence of the red libc_children. At head, spawn_children_cap fails with posix_spawn ENOMEM. With the fix reverted, the same failure point gives child 240 SIGSEGV. signal_sigchld_ignored passes in both. That case spawns without waiting and without SIG_IGN, so the changed code is not on its path (spawn_child calls forget_ended only under SIG_IGN). The shared ceiling is recorded in issues/root-in-memory-costs-its-whole-size.md. I accept it as independent.

Findings

No blockers.

SUGGESTION sdk/libc/src/spawn.rs:478-487: named holds indices into children, and forget then invalidates them. children[named[at]] in the Take arm is sound only because turn never returns Take when ignored, and that guarantee lives in another module. Either take the pid out before forget (for example, keep named as pids, or map Take(at) to children[named[at]].pid before mutating), or call forget only in the arms where it applies, so the coupling cannot break silently. A simpler shape would also work: if ignored { retain the unended (one read) ; named empty → ECHILD ; else watch }, otherwise the existing find. It needs no turn at all, at the cost of the host law. Either is acceptable.

SUGGESTION sdk/libc/src/spawn.rs:414: forget assumes ended.len() == children.len(). If ended is shorter, the code panics. If it is longer, the code over-subtracts UNWAITED without any error. An assert_eq! would make this a loud invariant, or the closure counter could become let mut ended = ended.iter(); children.retain(|_| !ended.next().unwrap()).

SUGGESTION issues/a-childs-end-is-an-event-and-a-parent-takes-its-children-down.md:171: the reflowed line is 85 columns, against the paragraph's ~79. The wording "and so is a wait deciding from one read of each child's end" can read as if the host test establishes the single read, which it does not (see the control above). Something like "and so is a wait's decision from one read of each child's end" would be more accurate.

Verdict

LAND

@Japabu
Japabu marked this pull request as ready for review October 22, 2026 22:55
@Japabu
Japabu added this pull request to the merge queue Oct 22, 2026
Merged via the queue into main with commit 551a788 Oct 22, 2026
8 checks passed
@Japabu
Japabu deleted the wt/toyos-proc-waitrace branch October 22, 2026 23:37
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.

1 participant