Fix cxa_thread_atexit() to properly register thread-local atexits #10854
Conversation
juj
force-pushed
the
fix_cxa_thread_atexit
branch
from
8ff4b0b to
9ded8c7
Compare
| setThreadStatus: function() {}, | ||
| #endif | ||
|
|
||
| runExitHandlers: function() { |
There was a problem hiding this comment.
please add a comment here that this is run on exit on each pthread, and on the main thread (I was confused before reading enough code to realize that - I hope I understood correctly?)
There was a problem hiding this comment.
Actually, how about renaming exitHandlers to threadExitHandlers as IIUC this is just for pthreads, and not the main thread?
There was a problem hiding this comment.
That works, although that will be a bit redundant. These were named PThread.exitHandlers before, so having PThread.threadExitHandlers is duplicating the thread name. Perhaps that is worth it though.
| #endif | ||
|
|
||
| // used in rust, clang when doing thread_local statics | ||
| __cxa_thread_atexit: 'pthread_cleanup_push', |
There was a problem hiding this comment.
pthread_cleanup_push has different semantics than __cxa_thread_atexit:
- When a thread terminates by calling pthread_exit(3), all clean-up
handlers are executed as described in the preceding point.
(Clean-up handlers are not called if the thread terminates by
performing a return from the thread start function.)
So for full correctness, pthread_cleanup_* should use a different stack of cleanup handlers than the thread exit handlers. I'm not sure if this is an issue.
There was a problem hiding this comment.
Will pthread_cleanup_push be available at all if !USE_PTHREADS? Should these thread_atexits just map to atexit if pthreads aren't available?
There was a problem hiding this comment.
Thanks @abigailbunyan for the excellent observation. By the comment you post, pthread_cleanup_push handlers should only be called if thread performs an exit without returning. This PR does not change behavior around that: Both before and after this PR, pthread_cleanup_push handlers are called on all kinds of exits. So let's deal with that as a followup.
pthread_cleanup_push is available in single-threaded programs, and goes via completely different implementation in library_pthread_stub.js - which does map to atexit already.
There was a problem hiding this comment.
Though now I realize routing directly to atexit in singlethreaded builds generates smaller code size, so using that.
|
I wonder if we can rely on the native libc++abi implementation. I've started experimenting with it here and it might be possible to use this in all cases: #11040 |
…d not process-global atexits. (Also microcleanup code a little)
juj
force-pushed
the
fix_cxa_thread_atexit
branch
from
9ded8c7 to
30de04e
Compare
4 participants
Fix cxa_thread_atexit() to properly register thread-local atexits, and not process-global atexits. (Also microcleanup code a little). Fixes #10809.