Skip to content

[pthreads] Initialize TLS on workers in MINIMAL_RUNTIME mode. #24646

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 2, 2025
+47 −5
Merged

[pthreads] Initialize TLS on workers in MINIMAL_RUNTIME mode. #24646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4fee4e2
[pthreads] Initialize TLS on workers in MINIMAL_RUNTIME mode.
brendandahl Jul 2, 2025
a5371b7
also init on main thread
brendandahl Jul 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions src/postamble_minimal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ function initRuntime(wasmExports) {
#endif

#if PTHREADS
if (ENVIRONMENT_IS_PTHREAD) return
PThread.tlsInitFunctions.push(wasmExports['_emscripten_tls_init']);
if (ENVIRONMENT_IS_PTHREAD) return;
#endif

#if WASM_WORKERS
Expand All @@ -66,10 +67,6 @@ function initRuntime(wasmExports) {
writeStackCookie();
#endif

#if PTHREADS
PThread.tlsInitFunctions.push(wasmExports['_emscripten_tls_init']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this not remove it from the main thread as well? (And the new code only re-adds it for pthreads)

Or is this not needed on the main thread? But from libpthread.js it looks like it is run there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does remove it from the main thread, which was not my intention. I'm amazed that worked. I guess there's not much testing for minimal+pthread.

Ideally this code would be in some shared spot that both minimal runtime + regular runtime would share, but I think that's going to be a bigger refactor.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a hole in testing, yeah... we do have pthreads tests for minimal runtime, but without TLS, I guess, until this PR.

#endif

<<< ATINITS >>>

#if hasExportedSymbol('__wasm_call_ctors')
Expand Down
35 changes: 35 additions & 0 deletions test/pthread/test_pthread_c_thread_local.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright 2025 The Emscripten Authors. All rights reserved.
// Emscripten is available under two separate licenses, the MIT license and the
// University of Illinois/NCSA Open Source License. Both these licenses can be
// found in the LICENSE file.

#include <pthread.h>
#include <stdlib.h>
#include <assert.h>
#include <stdio.h>

static _Thread_local int thread_local = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it perhaps make sense to do = 3 here and assert that thread_local starts with this value in moth main and in `thread_start?


void *thread_start(void *arg) {
thread_local = 1;
assert(thread_local == 1);
printf("thread_local=%d\n", thread_local);
return NULL;
}

int main() {
thread_local = 2;
pthread_t thread;
int rc;

rc = pthread_create(&thread, NULL, thread_start, NULL);
assert(rc == 0);

rc = pthread_join(thread, NULL);
assert(rc == 0);

printf("thread_local=%d\n", thread_local);
assert(thread_local == 2);
printf("done\n");
return 0;
}
3 changes: 3 additions & 0 deletions test/pthread/test_pthread_c_thread_local.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
thread_local=1
thread_local=2
done
7 changes: 7 additions & 0 deletions test/test_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2636,6 +2636,13 @@ def test_pthread_thread_local_storage(self):
self.set_setting('INITIAL_MEMORY', '300mb')
self.do_run_in_out_file_test('pthread/test_pthread_thread_local_storage.cpp')

@node_pthreads
@also_with_minimal_runtime
def test_pthread_c_thread_local(self):
if self.get_setting('MINIMAL_RUNTIME') and is_sanitizing(self.cflags):
self.skipTest('MINIMAL_RUNTIME + threads + asan does not work')
self.do_run_in_out_file_test('pthread/test_pthread_c_thread_local.c')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surely we already have tests that cover this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like test/pthread/test_pthread_tls.cpp is probably what we want. It happens to use C++ thread_local instead of the C's _Thread_local but I think it should the same thing from the POV of this PR.


@node_pthreads
def test_pthread_cleanup(self):
self.set_setting('PTHREAD_POOL_SIZE', 4)
Expand Down
Loading