casaroli opened a new pull request, #20929: URL: https://github.com/apache/nuttx/pull/20929
## Summary Two problems when the pthread options are off, which is the default of most configurations: - **Cleanup handlers with `CONFIG_TLS_NCLEANUP=0`.** There is no cleanup stack, and `pthread_cleanup_push()` and `pthread_cleanup_pop()` were empty macros. A program compiled and linked, but its cleanup handlers never ran: not at `pthread_exit()`, not at a cancellation, and not at `pthread_cleanup_pop(1)`, where POSIX requires the handler to run. This PR declares the two functions in every configuration. With `CONFIG_TLS_NCLEANUP=0` the library does not have them, so **a program that uses cleanup handlers does not link**. That is intended: if it linked, its handlers still could not run, so the program would only seem to work. A link error is better than handlers that are silently lost; the fix is `CONFIG_TLS_NCLEANUP` > 0. The read-write lock functions of libc push a handler only when the cleanup stack exists, as before. - **The cancel type without `CONFIG_CANCELLATION_POINTS`.** `pthread_cancel()` then acts at once and `pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED)` fails with `EINVAL`, but the old type that `pthread_setcanceltype()` returned was `PTHREAD_CANCEL_DEFERRED`. A program could not see that a cancellation can stop the thread at any point. It now returns `PTHREAD_CANCEL_ASYNCHRONOUS`, the type that applies. Documentation (separate commit): `08_pthread.rst` describes both. ## Impact - Code that calls `pthread_cleanup_push()` or `pthread_cleanup_pop()` with `CONFIG_TLS_NCLEANUP=0` no longer links (`undefined reference to pthread_cleanup_push`). Before, it built, and its handlers never ran. Fix: set `CONFIG_TLS_NCLEANUP` > 0. The in-tree users are guarded: the read-write locks of libc (in this PR) and ostest (already). - Without `CONFIG_CANCELLATION_POINTS`, `pthread_setcanceltype()` reports `PTHREAD_CANCEL_ASYNCHRONOUS` as the old type. - All build modes and architectures. ## Testing Test programs on `qemu-armv8a:knsh` (QEMU with HVF): - `pcleanup.c`: a thread pushes a handler and calls `pthread_exit()`; the handler must run. - `canceltype.c`: `pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, &old)` must either succeed with `old` = `DEFERRED` (with cancellation points) or fail with `EINVAL` and `old` = `ASYNCHRONOUS` (without). | | master | this PR | |---|---|---| | `pcleanup.c`, defaults (`CONFIG_TLS_NCLEANUP=0`) | links, the handler never runs (FAIL) | does not link: `undefined references: pthread_cleanup_push` | | `canceltype.c`, defaults (no cancellation points) | `EINVAL`, old = `DEFERRED` (FAIL) | `EINVAL`, old = `ASYNCHRONOUS` (PASS) | | `pcleanup.c` and `canceltype.c`, `CONFIG_TLS_NCLEANUP=8` + `CONFIG_CANCELLATION_POINTS` | PASS, PASS | PASS, PASS | | ostest, defaults, `qemu-armv8a:knsh` and `qemu-armv8a:nsh` | status 0 | status 0 | With `CONFIG_TLS_NCLEANUP=8` and `CONFIG_CANCELLATION_POINTS`, ostest on `qemu-armv8a:knsh` stops in its cancel test 6 (cancel a message queue wait) with an assertion at `arm64_fatal.c:696`, on master and with this PR in the same way; ostest on `qemu-armv8a:nsh` exits with status 0. Build, each OK: `qemu-armv8a:nsh` (flat, arm64), `qemu-armv8a:knsh` (kernel, arm64), `rv-virt:nsh64` (flat, rv64), `rv-virt:knsh64` (kernel, rv64), `rv-virt:pnsh64` (protected, rv64), `lm3s6965-ek:qemu-protected` (protected, armv7-m). No new warnings. `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]
