casaroli opened a new pull request, #21737: URL: https://github.com/apache/nuttx/pull/21737
depends-on: apache/nuttx/pull/21729 ## Summary On the simulator, a signal that an interrupt handler sends to the running task runs the signal handler of the task inside the interrupt handler. `up_schedule_sigaction()` calls `sigdeliver()` at once when the target is the running task, also when an interrupt handler calls it (for example the watchdog of `alarm()`). The handler then runs in `sim_doirq()`, in interrupt context. A handler that calls `exit()` exits the task from the interrupt handler and breaks the scheduler state. The next blocking call of the shell then fails `DEBUGASSERT(!is_idle_task(rtcb))` in `nxsem_wait_slow()` (`semaphore/sem_wait.c:257`, `task: sh`). The LTP test `pthread_spin_lock/3-2` does this: it locks a spin lock that is not initialized and leaves with `exit()` from its `SIGALRM` handler. Since #21693 (spin locks with atomics), the lock of the test spins until the alarm, so `sim/citest` now fails this case. `pthread_spin_lock/1-1` fails in the same way, with the old spin locks too (the CI skips it). Other architectures change the return context of the interrupt, so that the task runs the handler after the interrupt. The simulator now does the same: - `up_schedule_sigaction()` runs the handler at once only out of an interrupt handler. In an interrupt handler, it keeps `sigdeliver`. - `sim_doirq()` calls `sim_sigdeliver()` when the interrupted task resumes, as `up_switch_context()` already does for a task that it switches back in. A signal that another task sends to a task that an interrupt preempted now also runs when that task resumes, not at its next context switch. ## Impact The simulator only (`arch/sim`), all host platforms. Signal handlers no longer run in interrupt context. Builds without `CONFIG_ENABLE_ALL_SIGNALS` do not change. Other architectures, the kernel and libc do not change. No ABI change. No documentation change. ## Testing Linux x86_64 host, CI image `ghcr.io/apache/nuttx/apache-nuttx-ci-linux`, `sim:citest` with make, LTP cases run from nsh (`ltp_interfaces_pthread_spin_*`): | LTP case | master | this PR | |---|---|---| | `pthread_spin_lock_3_2` | handler prints `Test PASSED`, then `Assertion failed : at file: semaphore/sem_wait.c:257 task: sh` | PASS, exit status 0 | | `pthread_spin_lock_1_1` | handler prints `Test PASSED`, then the same assertion | PASS, exit status 0 | | `pthread_spin_lock_1_2`, `pthread_spin_trylock_1_1` | TIMEOUT | PASS, exit status 0 | | `destroy_1_1`, `destroy_3_1`, `init_1_1`, `init_4_1`, `lock_3_1`, `trylock_4_1`, `unlock_1_1`, `unlock_1_2` | PASS | PASS | | `unlock_3_1` (expects that a thread can unlock a lock it does not hold; the CI skips it) | FAIL (EPERM) | FAIL (EPERM) | | `ostest` | PASS, exit status 0 | PASS, exit status 0 | The two TIMEOUT cases on master do not fail themselves: the simulator had stopped at the assertion of the case before. With #21693 reverted on master, `pthread_spin_lock_3_2` passes (the old lock takes any lock word other than 1 as free, so the test exits before the alarm), and `pthread_spin_lock_1_1` fails with the same assertion. CI suite (ntfc 0.0.3, nuttx-ntfc-testing release-0.0.2, as the `Linux (sim-01)` job), `sim:citest`: | | master | this PR | |---|---|---| | ntfc summary | 1216 passed, 1 failed (`ltp_interfaces_pthread_spin_lock_3_2`: "Device crashed"), 298 skipped | 1217 passed, 0 failed, 298 skipped | `sim:smp` (4 CPUs) with `CONFIG_PTHREAD_SPINLOCKS`, built without `-Werror` (`ostest` there has a `-Wundef` error on master: `CONFIG_ARCH_INTERRUPTSTACK`): | Test | master | this PR | |---|---|---| | `ostest` | PASS, exit status 0 | PASS, exit status 0 | | copy of `pthread_spin_lock/3-2` as `hello`, lock word set to `0xa5` | `Assertion failed : at file: sched/sched_smp.c:229` on CPU2 | same | On SMP, the alarm stops at an older defect before it gets to the simulator: the timer interrupt on CPU2 queues the signal for the thread that runs on CPU1, and `nxsig_queue_action()` then calls `nxsched_smp_call_single()`, which must not run in an interrupt handler (`DEBUGASSERT(!up_interrupt_context())`). That code is common to all architectures and this PR does not change it. `-Werror` build of `sim:nsh` with `CONFIG_ENABLE_PARTIAL_SIGNALS` (no `sim_sigdeliver()`): builds. `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]
