sys/posix/pthread: newlib compatibility#17734
Conversation
gschorcht
added
CI: ready for build
gschorcht
force-pushed
the
sys/posix/pthread_newlib_compatibility
branch
from
bf6a823 to
55ba502
Compare
| /** | ||
| * @brief Datatype to supply to pthread_once(). | ||
| */ | ||
| typedef volatile int pthread_once_t; |
There was a problem hiding this comment.
Not really clear to me why pthread_once_t was declared to be volatile, propably to vaoid optimizing it out. But I don't see any reason why it shouldn't be optimized out.
When using a toolchain with built-in POSIX thread support, static C++ constructors use a static mutex variable which is initialized with `pthread_once` when first used. However, since RIOT's `pthread_once_t` type is different from that in newlib's `pthread`, which is assumed by GCC, RIOT crashes as soon as static constructors are used. Changing the `pthread_once_t` type to be compatible with newlib's `pthread_once_t` type solves the problem and allows the RIOT `pthread` modules to be used even with toolchains with built-in POSIX thread support.
gschorcht
force-pushed
the
sys/posix/pthread_newlib_compatibility
branch
from
55ba502 to
c09d9d8
Compare
| */ | ||
| typedef volatile int pthread_once_t; | ||
| typedef struct { | ||
| int is_initialized; /**< initialized */ |
There was a problem hiding this comment.
So this value is only touched by newlib?
There was a problem hiding this comment.
A variable of type pthread_once_t can be used by both user code and the gcc/g++ libraries to ensure that the specified function (the init_routine) is executed only once. However, the value of such a pthread_once_t variable must only be touched by the pthread_once() function. The state of a pthread_once_t variable should not be of interest for code usage.
Before a pthread_once_t variable is used, it must be initialized with PTHREAD_ONCE_INIT. Since the variable was previously a simple int variable, it would be theoretically possible for the user code to initialize it with = 0.But that should now trigger a compilation error.
benpicco
left a comment
benpicco
left a comment
There was a problem hiding this comment.
If Murdock is happy with this, this can go in.
|
Thanks for reviewing and merging. |
4 participants
Contribution description
This PR provides a
pthread_once_ttype that is compatible with newlib'spthread_once_t.When using a toolchain with built-in POSIX thread support, static C++ constructors use a static (fake) mutex variable which is initialized with
pthread_oncewhen first used. However, since RIOT'spthread_once_ttype is different from that in newlib'spthread, which is assumed by GCC, RIOT crashes as soon as static constructors are used.Changing the
pthread_once_ttype to be compatible with newlib'spthread_once_ttype solves the problem and allows the RIOTpthreadmodules to be used even with toolchains with built-in POSIX thread support. The costs for this change is one additionalintvariable.With this change, the precompiled ESP32 toolchains including OpenOCD from Espressif can be used and is no longer necessary to compile them especially for RIOT.
Found out while migrating the ESP32 port to ESP-IDF 4.4, which is the prerequisite for the RIOT port on all different ESP32 SoC variants.
Testing procedure
Compilation in CI should succeed and the
tests/pthread*andtests/cpp_ctorsshould still work. A succeeding test run in CI should be sufficient.Issues/PRs references
Prerequisite for PR #17601