Skip to content

Commit 662cb06

Browse files
sbc100tlively
andauthored
Remove redundant sleeping field from pthread struct. NFC (#26691)
The `sleeping` field was used to synchronize dlopen between threads, but it is now redundant since the implementation of precise futex wakeups in #26659. The `wait_addr` field now provides the same information (whether a thread is currently blocked). We now simply call `_emscripten_thread_notify` for every thread when proxying dlopen sync events. This ensures that threads process the dlopen catch-up queue promptly, even without the periodic 100ms wakeups that were removed in #26659. This change: 1. Removes the `sleeping` field from struct pthread. 2. Updates `emscripten_futex_wait` to no longer set/clear the sleeping field. 3. Removes a redundant `_emscripten_process_dlopen_queue` call in `emscripten_futex_wait`. (One remains at the end of the function, and one is called via `emscripten_yield` at the start). 4. Updates `dynlink.c` to use `_emscripten_thread_notify`. --------- Co-authored-by: Thomas Lively <tlively123@gmail.com>
1 parent b9e9469 commit 662cb06

File tree

3 files changed

+5
-20
lines changed

3 files changed

+5
-20
lines changed

system/lib/libc/dynlink.c

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <threads.h>
1717
#include <stdarg.h>
1818
#include <stdbool.h>
19+
#include <stdatomic.h>
1920
#include <stdio.h>
2021
#include <stdlib.h>
2122
#include <string.h>
@@ -29,6 +30,7 @@
2930

3031
#include "dynlink.h"
3132
#include "pthread_impl.h"
33+
#include "threading_internal.h"
3234
#include "emscripten_internal.h"
3335

3436
//#define DYLINK_DEBUG
@@ -406,13 +408,9 @@ int _emscripten_proxy_dlsync_async(pthread_t target_thread, em_promise_t promise
406408
emscripten_promise_resolve(promise, EM_PROMISE_FULFILL, NULL);
407409
emscripten_promise_destroy(promise);
408410
free(info);
409-
} else if (target_thread->sleeping) {
410-
// If the target thread is in the sleeping state (and this check is
411-
// performed after the enqueuing of the async work) then we know its safe to
412-
// resolve the promise early, since the thread will process our event as
413-
// soon as it wakes up.
414-
emscripten_promise_resolve(promise, EM_PROMISE_FULFILL, NULL);
415-
return 0;
411+
} else {
412+
// Wake up the target thread in case it's blocked in futex_wait
413+
_emscripten_thread_notify(target_thread);
416414
}
417415
return rtn;
418416
}

system/lib/libc/musl/src/internal/pthread_impl.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -122,12 +122,6 @@ struct pthread {
122122
// Since futex addresses must be 4-byte aligned, the low bit is safe to use.
123123
_Atomic uintptr_t wait_addr;
124124
#endif
125-
#ifdef EMSCRIPTEN_DYNAMIC_LINKING
126-
// When dynamic linking is enabled, threads use this to facilitate the
127-
// synchronization of loaded code between threads.
128-
// See emscripten_futex_wait.c.
129-
_Atomic char sleeping;
130-
#endif
131125
};
132126

133127
#ifdef __EMSCRIPTEN__

system/lib/pthread/emscripten_futex_wait.c

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -173,12 +173,6 @@ int emscripten_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms)
173173
max_wait_ns = (int64_t)(max_wait_ms * 1e6);
174174
}
175175
#ifdef __EMSCRIPTEN_PTHREADS__
176-
#ifdef EMSCRIPTEN_DYNAMIC_LINKING
177-
if (!is_runtime_thread) {
178-
self->sleeping = 1;
179-
_emscripten_process_dlopen_queue();
180-
}
181-
#endif
182176
uintptr_t expected_null = 0;
183177
if (atomic_compare_exchange_strong(&self->wait_addr, &expected_null, (uintptr_t)addr)) {
184178
DBG("emscripten_futex_wait atomic.wait ns=%lld", max_wait_ns);
@@ -197,7 +191,6 @@ int emscripten_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms)
197191
self->wait_addr = 0;
198192
#ifdef EMSCRIPTEN_DYNAMIC_LINKING
199193
if (!is_runtime_thread) {
200-
self->sleeping = 0;
201194
_emscripten_process_dlopen_queue();
202195
}
203196
#endif

0 commit comments

Comments
 (0)