Skip to content

Commit 3ee3c7a

Browse files
committed
Use native code for pthread_cleanup_push/pop. NFC
This change reverts one of our musl patches so that `pthread_cleanup_push/pop` are now implemented as macros as in upstream musl. `pthread_cleanup_push/pop` are now handled separately to `__cxa_atexit_thread`, which is technically a bug fix. Without this fix, calling `__cxa_atexit_thread` in the middle of push/pop block would end up pop'ing the wrong thing at the end. Overall this reduces the API surface of our JS API and increases the amount of code we share with musl, while also reducing the cost of `pthread_cleanup_push/pop.` Now, they never involve any allocation or calls to JS!
1 parent e210f98 commit 3ee3c7a

File tree

6 files changed

+40
-41
lines changed

6 files changed

+40
-41
lines changed

src/library.js

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -444,15 +444,6 @@ LibraryManager.library = {
444444
},
445445
__cxa_atexit: 'atexit',
446446

447-
#endif
448-
449-
// used in rust, clang when doing thread_local statics
450-
#if USE_PTHREADS
451-
__cxa_thread_atexit: 'pthread_cleanup_push',
452-
__cxa_thread_atexit_impl: 'pthread_cleanup_push',
453-
#else
454-
__cxa_thread_atexit: 'atexit',
455-
__cxa_thread_atexit_impl: 'atexit',
456447
#endif
457448

458449
// TODO: There are currently two abort() functions that get imported to asm module scope: the built-in runtime function abort(),

src/library_pthread.js

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,15 +1063,12 @@ var LibraryPThread = {
10631063
throw 'unwind';
10641064
},
10651065

1066-
pthread_cleanup_push__sig: 'vii',
1067-
pthread_cleanup_push: function(routine, arg) {
1068-
PThread.threadExitHandlers.push(function() { {{{ makeDynCall('vi', 'routine') }}}(arg) });
1066+
__cxa_thread_atexit__sig: 'vii',
1067+
__cxa_thread_atexit: function(routine, arg) {
1068+
PThread.threadExitHandlers.push(function() { {{{ makeDynCall('vi', 'routine') }}}(arg) });
10691069
},
1070+
__cxa_thread_atexit_impl: '__cxa_thread_atexit',
10701071

1071-
pthread_cleanup_pop: function(execute) {
1072-
var routine = PThread.threadExitHandlers.pop();
1073-
if (execute) routine();
1074-
},
10751072

10761073
// Returns 0 on success, or one of the values -ETIMEDOUT, -EWOULDBLOCK or -EINVAL on error.
10771074
emscripten_futex_wait__deps: ['emscripten_main_thread_process_queued_calls'],

src/library_pthread_stub.js

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,6 @@ var LibraryPThreadStub = {
1717
#endif
1818
},
1919

20-
pthread_cleanup_push__sig: 'vii',
21-
pthread_cleanup_push: function(routine, arg) {
22-
__ATEXIT__.push({ func: routine, arg: arg });
23-
_pthread_cleanup_push.level = __ATEXIT__.length;
24-
},
25-
26-
pthread_cleanup_pop__deps: ['pthread_cleanup_push'],
27-
pthread_cleanup_pop__sig: 'vi',
28-
pthread_cleanup_pop: function(execute) {
29-
assert(_pthread_cleanup_push.level == __ATEXIT__.length, 'cannot pop if something else added meanwhile!');
30-
var callback = __ATEXIT__.pop();
31-
if (execute) {
32-
{{{ makeDynCall('vi', 'callback.func') }}}(callback.arg)
33-
}
34-
_pthread_cleanup_push.level = __ATEXIT__.length;
35-
},
36-
3720
// When pthreads is not enabled, we can't use the Atomics futex api to do
3821
// proper sleeps, so simulate a busy spin wait loop instead.
3922
emscripten_thread_sleep__deps: ['emscripten_get_now'],
@@ -43,6 +26,9 @@ var LibraryPThreadStub = {
4326
// Do nothing.
4427
}
4528
},
29+
30+
__cxa_thread_atexit: 'atexit',
31+
__cxa_thread_atexit_impl: 'atexit',
4632
};
4733

4834
mergeInto(LibraryManager.library, LibraryPThreadStub);

system/lib/libc/musl/include/pthread.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -203,18 +203,11 @@ struct __ptcb {
203203
struct __ptcb *__next;
204204
};
205205

206-
#ifdef __EMSCRIPTEN__
207-
// For Emscripten, the cleanup stack is not implemented as a macro, since it's currently in the JS side.
208-
typedef void (*cleanup_handler_routine)(void *arg);
209-
void pthread_cleanup_push(cleanup_handler_routine routine, void *arg);
210-
void pthread_cleanup_pop(int execute);
211-
#else
212206
void _pthread_cleanup_push(struct __ptcb *, void (*)(void *), void *);
213207
void _pthread_cleanup_pop(struct __ptcb *, int);
214208

215209
#define pthread_cleanup_push(f, x) do { struct __ptcb __cb; _pthread_cleanup_push(&__cb, f, x);
216210
#define pthread_cleanup_pop(r) _pthread_cleanup_pop(&__cb, (r)); } while(0)
217-
#endif
218211

219212
#ifdef _GNU_SOURCE
220213
struct cpu_set_t;

system/lib/pthread/library_pthread.c

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -946,3 +946,34 @@ int emscripten_proxy_main(int argc, char** argv) {
946946
}
947947

948948
weak_alias(__pthread_testcancel, pthread_testcancel);
949+
950+
// Taking from part if __pthread_exit in from musl's pthread_create.c
951+
void __run_cleanup_handlers(void* _unused) {
952+
pthread_t self = __pthread_self();
953+
while (self->cancelbuf) {
954+
void (*f)(void *) = self->cancelbuf->__f;
955+
void *x = self->cancelbuf->__x;
956+
self->cancelbuf = self->cancelbuf->__next;
957+
f(x);
958+
}
959+
}
960+
961+
extern int __cxa_thread_atexit(void (*)(void *), void *, void *);
962+
963+
extern int __dso_handle;
964+
965+
// Copied from musl's pthread_create.c
966+
void __do_cleanup_push(struct __ptcb *cb) {
967+
struct pthread *self = __pthread_self();
968+
cb->__next = self->cancelbuf;
969+
self->cancelbuf = cb;
970+
}
971+
972+
// Copied from musl's pthread_create.c
973+
void __do_cleanup_pop(struct __ptcb *cb) {
974+
static thread_local bool registerd = false;
975+
if (!registerd) {
976+
__cxa_thread_atexit(__run_cleanup_handlers, NULL, &__dso_handle);
977+
}
978+
__pthread_self()->cancelbuf = cb->__next;
979+
}

tools/system_libs.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -751,7 +751,7 @@ def get_files(self):
751751
if self.is_mt:
752752
ignore += [
753753
'clone.c', '__lock.c',
754-
'pthread_cleanup_push.c', 'pthread_create.c',
754+
'pthread_create.c',
755755
'pthread_kill.c', 'pthread_sigmask.c',
756756
'__set_thread_area.c', 'synccall.c',
757757
'__syscall_cp.c', '__tls_get_addr.c',
@@ -781,6 +781,7 @@ def get_files(self):
781781
path_components=['system', 'lib', 'libc', 'musl', 'src', 'thread'],
782782
filenames=[
783783
'pthread_self.c',
784+
'pthread_cleanup_push.c',
784785
# C11 thread library functions
785786
'call_once.c',
786787
'tss_create.c',

0 commit comments

Comments
 (0)