casaroli opened a new pull request, #21216: URL: https://github.com/apache/nuttx/pull/21216
## Summary All pending signals of a task group are in one list, `tg_sigpendingq`. A signal sent to one thread (`pthread_kill()`, `tgkill()`) that the thread cannot take at once (it blocks the signal, or it waits in a system call of a protected or kernel build) waits in this list, marked with the TCB of the thread (`sigpend->tcb`). Two problems: - **sched/signal: Release the signals pending for a thread when it exits.** `nxsig_cleanup()` released only the lists in the TCB. The entry stayed in the list of the group with a pointer to the freed TCB: the signal was never delivered, its memory was lost, and a later thread at the same TCB address took it, out of turn. The new `nxsig_discard_threadsignals()` removes the entries of a thread from the list of its group; `nxtask_exithook()` calls it before the thread leaves its group. The exit sets `TCB_FLAG_EXIT_PROCESSING` in a critical section before that, and `nxsig_tcbdispatch()` now checks the flag again in the critical section where it queues the signal (it can leave the critical section to allocate), so no entry comes after the discard, also with SMP. - **sched/signal: Keep the pending signal of each target apart.** `nxsig_add_pendingsignal()` looked for an entry with the same signal number only. A second pending signal for another thread, or for the whole group, was merged into the entry of the first thread, and the other thread never got it. It now looks for an entry with the same signal number and the same target. POSIX keeps the pending signals of each thread and of the process apart; `sigpending()` still returns both for the calling thread. - One commit describes pending signals in `Documentation/reference/user/07_signals.rst`. How it shows on master: ostest on qemu-armv8a:knsh with `CONFIG_SIG_SIGUSR1_ACTION` does not finish. In `signest_test` a signal for the waiter thread runs its handler in the main thread of ostest (`waiter_action: Delivered to wrong TID 40, expected 99`), and the test asserts (`signest.c:256`, `signest.c:115`). Merge order: apache/nuttx-apps#3947 (`testing/ostest: Catch the SIGUSR1 that wakes up the mqueue receiver.`) first, then this PR. `mqueue_test` wakes up its receiver with SIGUSR1 and does not catch it. With this PR the receiver of the kernel build also gets the signal, so with `CONFIG_SIG_SIGUSR1_ACTION` the default action ends ostest in `mqueue_test`; the apps PR catches the signal. No CI configuration runs ostest with `CONFIG_SIG_SIGUSR1_ACTION`, so each PR builds and passes CI without the other. depends-on: apache/nuttx-apps/pull/3947 ## Impact Behaviour only, for signals that are pending for one thread: they are discarded when the thread exits, and each thread has its own pending instance. Nothing changes for a signal sent to the whole group or for real-time signals (they are queued). The number of pending entries for one signal number is now at most one per thread plus one for the group. No change to a structure or an interface. All build modes and architectures; the code is not built with `CONFIG_DISABLE_ALL_SIGNALS`. Not a breaking change, also for the flat build: no interface, structure or configuration changes. Only a signal that was lost or delivered to the wrong thread is now delivered as POSIX requires. ## Testing QEMU with HVF, `CONFIG_SIG_SIGUSR1_ACTION`, `CONFIG_SCHED_CHILD_STATUS` and `CONFIG_CANCELLATION_POINTS`; the flat builds also with `CONFIG_SIG_DEFAULT`. The tests ran on 23f5a4f06df: master 7505b874722 (#21072) does not build for arm64 (`lib_libvsprintf.c:219` and `:232`, `-Werror=shadow`; fixed by #21210). Test program `sigpendthr`, the same results on qemu-armv8a:knsh (kernel), qemu-armv8a:nsh (flat) and qemu-armv8a:nsh_smp (flat, SMP, 2 CPUs): | Case | master | this PR | |---|---|---| | merge: two threads block SIGUSR2, each gets one with `pthread_kill()`, the process gets one with `kill()`; then all unblock (handled by thread 1, thread 2, main; want >= 1, >= 1 and 3 in all) | 1, 0, 0: FAIL | 2, 1, 0: PASS | | wait: a thread blocks SIGUSR2 and gets one with `pthread_kill()`, the process one with `kill()`; `sigtimedwait()` with a zero timeout in the thread (want 2 signals) | 1: FAIL | 2: PASS | | exit: 10 times, a thread that blocks SIGUSR1 gets it with `pthread_kill()` and exits, then a new thread that does not block SIGUSR1 runs (want 0 SIGUSR1 in new threads) | 10: FAIL | 0: PASS | ostest exit status: | NuttX | apps | qemu-armv8a:knsh | qemu-armv8a:nsh | qemu-armv8a:nsh + `CONFIG_SIG_DEFAULT` | |---|---|---|---|---| | master | master | no end: assertion in `signest_test` (above) | 0 | 256: ends in `mqueue_test` | | master | the apps PR | 0 | 0 | 0 | | this PR | master | 256: ends in `mqueue_test` (SIGUSR1 default action) | 0 | 256: ends in `mqueue_test` | | this PR | the apps PR | 0 | 0 | 0 | Rows 2 and 3 are from the earlier base 3122150b1f1; rows 1 and 4 are on 23f5a4f06df. qemu-armv8a:nsh_smp (2 CPUs) + `CONFIG_SIG_DEFAULT` with the apps PR, master and this PR: every test up to the spinlock test passes, and then the spinlock test (10000000 loops) does not end in 20 minutes on both (not changed here). Build, each OK: qemu-armv8a:nsh (flat), also with `CONFIG_ENABLE_PARTIAL_SIGNALS`; qemu-armv8a:nsh_smp (flat, SMP); qemu-armv8a:knsh (kernel); rv-virt:pnsh64 (protected, rv64); rv-virt:knsh64 and rv-virt:ksmp64 (kernel, rv64, SMP); mps2-an500:knsh (protected, Cortex-M7). No new warning. With `CONFIG_DISABLE_ALL_SIGNALS`, qemu-armv8a:nsh compiles the kernel and then fails to link in apps: `system/nxinit` calls `sigprocmask()` and `ppoll()`, which this configuration does not have (not changed here). `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]
