casaroli opened a new pull request, #21352: URL: https://github.com/apache/nuttx/pull/21352
## Summary With `CONFIG_SCHED_CHILD_STATUS`, `SA_NOCLDWAIT` and `SIGCHLD` set to `SIG_IGN` did not follow POSIX (XSI): - `sigaction(SIGCHLD)` with `SA_NOCLDWAIT` set `GROUP_FLAG_NOCLDWAIT`, but no call cleared it. A process that used `SA_NOCLDWAIT` once never kept the exit status of a child again. The old action that `sigaction()` returned did not have `SA_NOCLDWAIT`, so a save and restore also lost the setting. - XSI gives `SIGCHLD` set to `SIG_IGN` the same effect as `SA_NOCLDWAIT`. NuttX kept the exit status in this case. - Without retention, `waitpid()` and `waitid()` returned the PID and the status of the first child that exited. XSI says that the call blocks until all children of the process exit, and then fails with `ECHILD`. - The child of `fork()` or `vfork()` gets the signal actions of its parent (`nxsig_dupactions()`), but not the flag. A child that inherited `SIG_IGN` or `SA_NOCLDWAIT` for `SIGCHLD` still kept the status of its own children. - If another thread set `SA_NOCLDWAIT` while `waitpid(pid)` or `waitid(P_PID)` waited, the wait loop did not find the child status structure of the waited-for child any more (`DEBUGASSERT`, then a NULL pointer access). The first commit makes `sigaction(SIGCHLD)` set the flag for `SA_NOCLDWAIT` or `SIG_IGN` and clear it for all other actions; the old action has `SA_NOCLDWAIT` when the flag is set. `SIG_IGN` counts only with `CONFIG_SIG_DEFAULT`, because without it `SIG_IGN` and `SIG_DFL` have the same value. The flag changes only after the new action is in place, so a `sigaction()` that fails with `ENOMEM` changes nothing. `nxsig_dupactions()` now also copies the flag to the child of `fork()` or `vfork()`. The second commit adds `group_has_child()`, which finds a running child (any child or one PID) and skips the threads in exit processing. Without retention, `waitpid()` and `waitid()` retry after each `SIGCHLD` until no child is left, then fail with `ECHILD`. A wait for one PID used `nxsched_get_tcb(pid)`, which also finds a child that is in its exit, so a retry after the signal could wait for ever. `group_has_child()` finds only a child process with that PID, so a negative PID still fails with `ECHILD`. The wait loops read the flag again after each signal, and a wait for a child without a child status structure continues while the child runs and then fails with `ECHILD`. The third commit updates the `sigaction()` differences and the child status section of the task control page. ## Impact Only configurations with `CONFIG_SCHED_HAVE_PARENT` and `CONFIG_SCHED_CHILD_STATUS` (the default in kernel builds), in all build modes. Without `CONFIG_SCHED_CHILD_STATUS` nothing changes. - A program that sets `SA_NOCLDWAIT` (or `SIGCHLD` to `SIG_IGN`), or a `fork()` child of such a program, and then calls `wait()`, `waitpid()`, `waitid()` or `pclose()` gets `ECHILD` after its children exit, not the status of one child. ostest already accepts `ECHILD` there. - A program that sets `SA_NOCLDWAIT` for a while and then sets another action for `SIGCHLD` keeps the exit status of later children again, so it must wait for them. For example, ostest sets `SA_NOCLDWAIT` at the start, and its `sighand_test` sets a `SIGCHLD` handler and then `SIG_DFL`: the tests after it keep the child status. NuttX still decides when a child is created whether its status is kept (the child status structure is allocated only while the flag is clear). A child that starts while the flag is set and exits after it is cleared does not become waitable. XSI decides at the exit of the child. This PR does not change that, and the documentation says it. ## Testing New test program `nocldwait.c` (eleven cases, run in this order in one process) on `qemu-armv8a:knsh` (kernel build, QEMU with HVF; children from `posix_spawn()` and `vfork()`) and on `qemu-armv8a:nsh` (flat build; children from `task_create()` and `vfork()`), both with `CONFIG_SCHED_WAITPID`, `CONFIG_SCHED_CHILD_STATUS` and `CONFIG_SIG_DEFAULT` (knsh also with `CONFIG_PIPES` for case 11). Cases 1 to 10 give the same results on both builds; case 11 (`fork()`) runs on knsh only: | Case | master | this PR | |---|---|---| | 1 control: `waitpid()` gets the status | PASS | PASS | | 2 `SA_NOCLDWAIT`, then a handler without it: status kept | FAIL (`ECHILD`) | PASS | | 3 `SIG_IGN`: `waitpid(pid)` fails with `ECHILD` | PASS (only because the flag of case 2 stayed set) | PASS | | 4 `SIG_IGN`: `waitpid(-1)` waits for all children (200 ms and 1000 ms), then `ECHILD` | FAIL (returned a PID after 200 to 250 ms) | PASS (`ECHILD` after about 1000 ms) | | 5 the old action has `SA_NOCLDWAIT`; a restore keeps it | FAIL | PASS | | 6 `SIG_DFL`: status kept | FAIL (`ECHILD`) | PASS | | 7 `SIG_IGN`: `waitid(P_ALL)` waits for all children (200 ms and 1000 ms), then `ECHILD` | FAIL (returned a child) | PASS (`ECHILD` after about 1000 ms) | | 8 `SIG_IGN`: `waitpid(-1, WNOHANG)` gives 0 while a child runs, `ECHILD` after it exits | FAIL (0 after the exit) | PASS | | 9 `vfork()` child: status kept with `SIG_DFL`, `ECHILD` with `SIG_IGN` | FAIL (`ECHILD` with `SIG_DFL`) | PASS | | 10 while `waitpid(pid)` waits, another thread sets `SA_NOCLDWAIT`, kills another child, and kills the waited-for child 300 ms later: wait for the child, then `ECHILD` | FAIL (returned the PID; the flag of case 2 stayed set) | PASS (`ECHILD` after the waited-for child ends) | | 11a (knsh) a `fork()` child of a parent with `SIGCHLD` set to `SIG_IGN` forks a grandchild: after its exit, `waitpid(grandchild)` and `waitpid(-1, WNOHANG)` fail with `ECHILD` | FAIL (`waitpid(grandchild)` returned its PID) | PASS | | 11b (knsh) the same with `SA_NOCLDWAIT` in the parent | FAIL (`waitpid(grandchild)` returned its PID: the parent had the flag, the child did not) | PASS | Case 10 in a new process (`nocldrace.c`, knsh: the status is kept at the start; while `waitpid(pid)` waits, a thread sets `SA_NOCLDWAIT`, kills the other child, and kills the waited-for child 300 ms later): master stops with `Assertion failed : at file: sched/sched_waitpid.c:117` (`DEBUGASSERT(child)`); with this PR `waitpid()` fails with `ECHILD` after the waited-for child ends. Without `CONFIG_SCHED_CHILD_STATUS` (`qemu-armv8a:knsh`), master and this PR give the same results (cases 3, 5 (restore), 8 (running) and 9 (`SIG_IGN`) pass, the others fail as expected without retention). `killpgrp.c` (the test of `kill()` to a process group: `waitpid(-pid, WNOHANG)` and `waitid(P_PID, -pid, WNOHANG)` with `SA_NOCLDWAIT` fail with `ECHILD`) passes 11/11 on master and with this PR, knsh and flat. ostest exits with status 0 on master and with this PR: `qemu-armv8a:knsh` and `qemu-armv8a:nsh` (with the options above), and the flat `qemu-armv8a:nsh` run with the test programs; also `qemu-armv8a:knsh` without `CONFIG_SCHED_CHILD_STATUS` (base 2e6e1b0a2ad). Build, each OK (`-Werror`, no warnings in `sched/`): `qemu-armv8a:nsh` (flat, arm64), the same with `CONFIG_DISABLE_PTHREAD` and `CONFIG_SCHED_CHILD_STATUS`, `qemu-armv8a:nsh_smp` with `CONFIG_SCHED_CHILD_STATUS` and `CONFIG_DEBUG_ASSERTIONS`, `qemu-armv8a:knsh` (kernel, arm64), `mps2-an500:knsh` (protected, Cortex-M7) with `CONFIG_SCHED_CHILD_STATUS` and `CONFIG_SIG_DEFAULT`, `mps2-an500:nsh` (flat, Cortex-M7) with `CONFIG_SCHED_HAVE_PARENT` and without `CONFIG_SCHED_CHILD_STATUS`, `rv-virt:nsh64` (flat, rv64), also with `CONFIG_SCHED_HAVE_PARENT` and without `CONFIG_SCHED_CHILD_STATUS`, `rv-virt:pnsh64` (protected, rv64) and `rv-virt:knsh64` (kernel, rv64) with `CONFIG_SCHED_CHILD_STATUS`. Base: master 1b0691e870d. The results without `CONFIG_SCHED_CHILD_STATUS` are from the base 2e6e1b0a2ad (before #21144; the rebase changed only the lines that #21144 also changed). `tools/checkpatch.sh -c -u -m -g` passes. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
