Skip to content

Commit b347ced

Browse files
sbc100sbc100
authored
Fix misues of is_main_runtime_thread over is_main_browser_thread (emscripten-core#15503)
In all these places the code is really attempting to figure out if it is the main runtime thread. i.e. the first place the program is loaded and the place that runs the callback and async events send from secondary threads. The reason this mistake often goes unnoticed is that in almost all cases the main runtime thread is also running on the main browser thread. One easy way to see that `is_main_browser_thread` is the wrong question to be asking in many of these cases is to remember that when emscripten is started in a worker there is no main browser involved and so this function will return false on *all* threads. In the cast of `__timedwait.c` and `pthread_barrier_wait.c` the desire is to avoid blocking the main runtime threads so that calls from other threads can be processed by `emscripten_main_thread_process_queued_calls`. `emscripten_main_thread_process_queued_calls` is expected to always run on the main runtime thread, and not necessarily on the main browser thread. Indeed its first line is: `assert(emscripten_is_main_runtime_thread());`
1 parent 9b98d42 commit b347ced

7 files changed

Lines changed: 17 additions & 17 deletions

File tree

‎system/lib/libc/musl/src/thread/__timedwait.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,11 @@ int __timedwait_cp(volatile int *addr, int val,
6464

6565
#ifdef __EMSCRIPTEN__
6666
double msecsToSleep = top ? (top->tv_sec * 1000 + top->tv_nsec / 1000000.0) : INFINITY;
67-
int is_main_thread = emscripten_is_main_browser_thread();
67+
int is_runtime_thread = emscripten_is_main_runtime_thread();
6868
// cp suffix in the function name means "cancellation point", so this wait can be cancelled
6969
// by the users unless current threads cancelability is set to PTHREAD_CANCEL_DISABLE
7070
// which may be either done by the user of __timedwait() function.
71-
if (is_main_thread ||
71+
if (is_runtime_thread ||
7272
pthread_self()->canceldisable != PTHREAD_CANCEL_DISABLE ||
7373
pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) {
7474
double sleepUntilTime = emscripten_get_now() + msecsToSleep;
@@ -80,15 +80,15 @@ int __timedwait_cp(volatile int *addr, int val,
8080
return ECANCELED;
8181
}
8282
// Assist other threads by executing proxied operations that are effectively singlethreaded.
83-
if (is_main_thread) emscripten_main_thread_process_queued_calls();
83+
if (is_runtime_thread) emscripten_main_thread_process_queued_calls();
8484
// Must wait in slices in case this thread is cancelled in between.
8585
double waitMsecs = sleepUntilTime - emscripten_get_now();
8686
if (waitMsecs <= 0) {
8787
r = ETIMEDOUT;
8888
break;
8989
}
9090
if (waitMsecs > 100) waitMsecs = 100; // non-main threads can sleep in longer slices.
91-
if (is_main_thread && waitMsecs > 1) waitMsecs = 1; // main thread may need to run proxied calls, so sleep in very small slices to be responsive.
91+
if (is_runtime_thread && waitMsecs > 1) waitMsecs = 1; // the runtime thread may need to run proxied calls, so sleep in very small slices to be responsive.
9292
r = -emscripten_futex_wait((void*)addr, val, waitMsecs);
9393
} while(r == ETIMEDOUT);
9494
} else {

‎system/lib/libc/musl/src/thread/__wait.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ void __wait(volatile int *addr, volatile int *waiters, int val, int priv)
1919
}
2020
if (waiters) a_inc(waiters);
2121
#ifdef __EMSCRIPTEN__
22-
int is_main_thread = emscripten_is_main_runtime_thread();
22+
int is_runtime_thread = emscripten_is_main_runtime_thread();
2323
while (*addr==val) {
24-
if (is_main_thread || pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) {
24+
if (is_runtime_thread || pthread_self()->cancelasync == PTHREAD_CANCEL_ASYNCHRONOUS) {
2525
// Must wait in slices in case this thread is cancelled in between.
2626
int e;
2727
do {
@@ -30,10 +30,10 @@ void __wait(volatile int *addr, volatile int *waiters, int val, int priv)
3030
return;
3131
}
3232
// Assist other threads by executing proxied operations that are effectively singlethreaded.
33-
if (is_main_thread) emscripten_main_thread_process_queued_calls();
33+
if (is_runtime_thread) emscripten_main_thread_process_queued_calls();
3434
// Main thread waits in _very_ small slices so that it stays responsive to assist proxied
3535
// pthread calls.
36-
e = emscripten_futex_wait((void*)addr, val, is_main_thread ? 1 : 100);
36+
e = emscripten_futex_wait((void*)addr, val, is_runtime_thread ? 1 : 100);
3737
} while(e == -ETIMEDOUT);
3838
} else {
3939
// Can wait in one go.

‎system/lib/libc/musl/src/thread/pthread_barrier_wait.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,9 @@ int pthread_barrier_wait(pthread_barrier_t *b)
8888
a_spin();
8989
a_inc(&inst->finished);
9090
#ifdef __EMSCRIPTEN__
91-
int is_main_thread = emscripten_is_main_browser_thread();
91+
int is_runtime_thread = emscripten_is_main_runtime_thread();
9292
while (inst->finished == 1) {
93-
if (is_main_thread) {
93+
if (is_runtime_thread) {
9494
int e;
9595
do {
9696
// Main thread waits in _very_ small slices so that it stays responsive to assist proxied

‎system/lib/pthread/library_pthread.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ void emscripten_thread_sleep(double msecs) {
9191
// If we have less than this many msecs left to wait, busy spin that instead.
9292
const double minimumTimeSliceToSleep = 0.1;
9393

94-
// main thread may need to run proxied calls, so sleep in very small slices to be responsive.
95-
const double maxMsecsSliceToSleep = emscripten_is_main_browser_thread() ? 1 : 100;
94+
// runtime thread may need to run proxied calls, so sleep in very small slices to be responsive.
95+
const double maxMsecsSliceToSleep = emscripten_is_main_runtime_thread() ? 1 : 100;
9696

9797
emscripten_conditional_set_current_thread_status(
9898
EM_THREAD_STATUS_RUNNING, EM_THREAD_STATUS_SLEEPING);

‎system/lib/pthread/pthread_join.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ static int __pthread_join_internal(pthread_t t, void **res) {
3030
// thread is attempting to join to itself.
3131
return EDEADLK;
3232
}
33-
int is_main_thread = emscripten_is_main_runtime_thread();
33+
int is_runtime_thread = emscripten_is_main_runtime_thread();
3434
while (1) {
3535
// The thread we are joining with must be either DT_JOINABLE or
3636
// DT_EXITING. If its DT_EXITING then we move it to DT_EXITED and
@@ -49,8 +49,8 @@ static int __pthread_join_internal(pthread_t t, void **res) {
4949
// In main runtime thread (the thread that initialized the Emscripten C
5050
// runtime and launched main()), assist pthreads in performing operations
5151
// that they need to access the Emscripten main runtime for.
52-
if (is_main_thread) emscripten_main_thread_process_queued_calls();
53-
emscripten_futex_wait(&t->detach_state, old_state, is_main_thread ? 100 : 1);
52+
if (is_runtime_thread) emscripten_main_thread_process_queued_calls();
53+
emscripten_futex_wait(&t->detach_state, old_state, is_runtime_thread ? 100 : 1);
5454
}
5555
}
5656

‎system/lib/wasmfs/file.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ void MemoryFile::Handle::preloadFromJS(int index) {
4545
getFile()->buffer.resize(
4646
EM_ASM_INT({return wasmFS$preloadedFiles[$0].fileData.length}, index));
4747
// Ensure that files are preloaded from the main thread.
48-
assert(emscripten_is_main_browser_thread());
48+
assert(emscripten_is_main_runtime_thread());
4949
// TODO: Replace every EM_ASM with EM_JS.
5050
EM_ASM({ HEAPU8.set(wasmFS$preloadedFiles[$1].fileData, $0); },
5151
getFile()->buffer.data(),

‎system/lib/wasmfs/wasmfs.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ void WasmFS::preloadFiles() {
5353
#endif
5454

5555
// Ensure that files are preloaded from the main thread.
56-
assert(emscripten_is_main_browser_thread());
56+
assert(emscripten_is_main_runtime_thread());
5757

5858
int numFiles = EM_ASM_INT({return wasmFS$preloadedFiles.length});
5959
int numDirs = EM_ASM_INT({return wasmFS$preloadedDirs.length});

0 commit comments

Comments
 (0)