Skip to content

Commit 47e7d67

Browse files
authored
Update emscripten_futex_wait to return -EINTR on spurious wakeups. NFC (#26735)
The internal musl code assumes that futex wait returns -EINTR if the wait is interrupted. We return -EINTR here in case we wakeup to run a timer or if were notified to wake up by another thread using `emscripten_thread_notify`. Followup to #26659
1 parent 3022279 commit 47e7d67

13 files changed

Lines changed: 144 additions & 49 deletions

ChangeLog.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ See docs/process.md for more on how version tagging works.
3131
emscripten so folks can sill use it manually if needed. If you are a user of
3232
this feature please let us know otherwise this may be deleted in a future
3333
release. (#26648)
34+
- The emscripten_futux_wait internals were improved to avoid unnecessary thread
35+
wakeups. As part of this change emscripten_futux_wait is now documented to
36+
explicitly allow for returning EINTR when the wait is interrupted by an async
37+
operation. All callers of emscripten_futux_wait are advised to use a loop
38+
to handle these types of spurious wakeups / interruptions. (#26659, #26735)
3439

3540
5.0.6 - 04/14/26
3641
----------------

system/include/emscripten/threading_primitives.h

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -187,12 +187,13 @@ ATOMICS_WAIT_TOKEN_T emscripten_condvar_wait_async(emscripten_condvar_t * _Nonnu
187187
void emscripten_condvar_signal(emscripten_condvar_t * _Nonnull condvar, uint32_t numWaitersToSignal);
188188

189189
// If the given memory address contains value val, puts the calling thread to
190-
// sleep waiting for that address to be notified.
191-
// Note: Like the Linux futex syscall, this API *does* allow spurious wakeups.
192-
// This differs from the WebAssembly `atomic.wait` instruction itself which
193-
// does *not* allow supurious wakeups and it means that most callers will want
194-
// to wrap this some kind of loop.
190+
// sleep waiting for that address to be notified. Like the linux futex syscall
191+
// this function returns negative errno values on failure.
195192
// Returns -EINVAL if addr is null.
193+
// Returns -ETIMEOUT if the maxWaitMilliseconds timeout was exceeded.
194+
// Returns -EINTR if the operation was interrupted (e.g. a timer fired, or an
195+
// async signal was received).
196+
// Returns 0 on success (i.e. another thread signaled this address)
196197
int emscripten_futex_wait(volatile void/*uint32_t*/ * _Nonnull addr, uint32_t val, double maxWaitMilliseconds);
197198

198199
// Wakes the given number of threads waiting on a location. Pass count ==

system/lib/libc/emscripten_yield_stub.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,14 @@
77

88
#include <features.h>
99

10-
static void dummy(double now) {
10+
#include "threading_internal.h"
11+
12+
static bool dummy(double now) {
13+
return false;
1114
}
1215

1316
weak_alias(dummy, _emscripten_check_timers);
1417

15-
void _emscripten_yield(double now) {
16-
_emscripten_check_timers(now);
18+
bool _emscripten_yield(double now) {
19+
return _emscripten_check_timers(now);
1720
}

system/lib/libc/musl/src/signal/setitimer.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,21 +64,25 @@ void _emscripten_timeout(int which, double now)
6464
raise(signum);
6565
}
6666

67-
void _emscripten_check_timers(double now)
67+
bool _emscripten_check_timers(double now)
6868
{
6969
// Timers always run on the main runtime thread. They are registered with
7070
// _setitimer_js which is proxied to the main runtime thread.
7171
assert(emscripten_is_main_runtime_thread());
72+
bool rtn = false;
7273
for (int which = 0; which < 3; which++) {
7374
if (current_timeout_ms[which]) {
7475
// Only call out to JS to get the current time if it was not passed in
7576
// *and* we have one or more timers set.
7677
if (!now)
7778
now = emscripten_get_now();
78-
if (now >= current_timeout_ms[which])
79+
if (now >= current_timeout_ms[which]) {
80+
rtn = true;
7981
_emscripten_timeout(which, now);
82+
}
8083
}
8184
}
85+
return rtn;
8286
}
8387

8488
double _emscripten_next_timer()

system/lib/pthread/emscripten_futex_wait.c

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@
2121
#include <stdlib.h>
2222
#include <sys/param.h>
2323

24+
#ifdef __EMSCRIPTEN_PTHREADS__
25+
// Note: We use a weak reference here. If it's null we know that threads are
26+
// not cancelable.
27+
weak long __cancel(void);
28+
#endif
29+
2430
extern void* _emscripten_main_thread_futex;
2531

2632
static int futex_wait_main_browser_thread(volatile void* addr,
@@ -53,8 +59,7 @@ static int futex_wait_main_browser_thread(volatile void* addr,
5359
while (1) {
5460
#ifdef __EMSCRIPTEN_PTHREADS__
5561
if (cancelable && __pthread_self()->cancel) {
56-
__pthread_testcancel();
57-
return -ETIMEDOUT;
62+
return __cancel();
5863
}
5964
#endif
6065
// Check for a timeout.
@@ -79,7 +84,10 @@ static int futex_wait_main_browser_thread(volatile void* addr,
7984
// We were told to stop waiting, so stop.
8085
break;
8186
}
82-
_emscripten_yield(now);
87+
bool timer_fired = _emscripten_yield(now);
88+
if (timer_fired) {
89+
return -EINTR;
90+
}
8391

8492
// Check the value, as if we were starting the futex all over again.
8593
// This handles the following case:
@@ -132,27 +140,24 @@ static double dummy() {
132140

133141
weak_alias(dummy, _emscripten_next_timer);
134142

135-
int emscripten_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms) {
143+
static int _do_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms) {
136144
if ((((intptr_t)addr)&3) != 0) {
137145
return -EINVAL;
138146
}
139147

140148
int ret;
141-
emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_RUNNING, EM_THREAD_STATUS_WAITFUTEX);
142149

143150
#ifdef __EMSCRIPTEN_PTHREADS__
144151
pthread_t self = __pthread_self();
145-
bool cancelable = self->canceldisable != PTHREAD_CANCEL_DISABLE;
152+
bool cancelable = __cancel && self->canceldisable != PTHREAD_CANCEL_DISABLE;
146153
#else
147154
bool cancelable = false;
148155
#endif
149156

150157
// For the main browser thread and audio worklets we can't use
151158
// __builtin_wasm_memory_atomic_wait32 so we have busy wait instead.
152159
if (!_emscripten_thread_supports_atomics_wait()) {
153-
ret = futex_wait_main_browser_thread(addr, val, max_wait_ms, cancelable);
154-
emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_WAITFUTEX, EM_THREAD_STATUS_RUNNING);
155-
return ret;
160+
return futex_wait_main_browser_thread(addr, val, max_wait_ms, cancelable);
156161
}
157162

158163
DBG("emscripten_futex_wait ms=%f", max_wait_ms);
@@ -182,28 +187,31 @@ int emscripten_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms)
182187
}
183188

184189
// Clear the wait_addr
185-
DBG("emscripten_futex_wait done notify=%d cancelable=%d cancel=%d", !!(self->wait_addr & NOTIFY_BIT), cancelable, self->cancel);
186-
self->wait_addr = 0;
190+
bool notified = atomic_exchange(&self->wait_addr, 0) & NOTIFY_BIT;
191+
192+
// Here we are mimicking the behaviour of musl's __syscall_cp_c which wraps
193+
// the linux futex syscall.
194+
if (self->cancel && cancelable) {
195+
return __cancel();
196+
}
197+
198+
DBG("emscripten_futex_wait done notified=%d cancelable=%d cancel=%d", notified, cancelable, self->cancel);
187199

188200
// Pass 0 here, which means we don't have access to the current time in this
189201
// function. This tells _emscripten_yield to call emscripten_get_now if (and
190202
// only if) it needs to know the time.
191-
_emscripten_yield(0);
192-
193-
if (cancelable && self->cancel) {
194-
__pthread_testcancel();
195-
// If __pthread_testcancel does return here it means that canceldisable
196-
// must be set to PTHREAD_CANCEL_MASKED. In this case we emulate the
197-
// behaviour of the futex syscall and return ECANCELLED here.
198-
// See pthread_cond_timedwait.c for the only use of this flag.
199-
emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_WAITFUTEX, EM_THREAD_STATUS_RUNNING);
200-
return -ECANCELED;
203+
bool timer_fired = _emscripten_yield(0);
204+
if (notified || timer_fired) {
205+
return -EINTR;
201206
}
202207
#else // __EMSCRIPTEN_PTHREADS__
203208
ret = __builtin_wasm_memory_atomic_wait32((int*)addr, val, max_wait_ns);
209+
bool timer_fired = _emscripten_yield(0);
210+
if (timer_fired) {
211+
return -EINTR;
212+
}
204213
#endif // __EMSCRIPTEN_PTHREADS__
205214

206-
emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_WAITFUTEX, EM_THREAD_STATUS_RUNNING);
207215

208216
if (ret == ATOMICS_WAIT_NOT_EQUAL) {
209217
return -EWOULDBLOCK;
@@ -214,3 +222,11 @@ int emscripten_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms)
214222
assert(ret == ATOMICS_WAIT_OK);
215223
return 0;
216224
}
225+
226+
227+
int emscripten_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms) {
228+
emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_RUNNING, EM_THREAD_STATUS_WAITFUTEX);
229+
int ret = _do_futex_wait(addr, val, max_wait_ms);
230+
emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_WAITFUTEX, EM_THREAD_STATUS_RUNNING);
231+
return ret;
232+
}

system/lib/pthread/emscripten_yield.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,14 @@ void _emscripten_thread_crashed() {
1818
_emscripten_thread_notify(emscripten_main_runtime_thread_id());
1919
}
2020

21-
static void dummy(double now)
22-
{
21+
static bool dummy(double now) {
22+
return false;
2323
}
2424

2525
weak_alias(dummy, _emscripten_check_timers);
2626

27-
void _emscripten_yield(double now) {
27+
bool _emscripten_yield(double now) {
28+
bool rtn = false;
2829
// When a secondary thread crashes, we need to be able to interrupt the main
2930
// thread even if it's in a blocking/looping on a mutex. We want to avoid
3031
// using the normal proxying mechanism to send this message since it can
@@ -42,7 +43,7 @@ void _emscripten_yield(double now) {
4243
}
4344

4445
// This is no-op in programs that don't include use of itimer/alarm.
45-
_emscripten_check_timers(now);
46+
rtn = _emscripten_check_timers(now);
4647

4748
// Assist other threads by executing proxied operations that are effectively
4849
// singlethreaded.
@@ -53,4 +54,5 @@ void _emscripten_yield(double now) {
5354
_emscripten_process_dlopen_queue();
5455
}
5556
#endif
57+
return rtn;
5658
}

system/lib/pthread/threading_internal.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#pragma once
99

1010
#include <pthread.h>
11+
#include <stdbool.h>
1112

1213
// Defined THREADING_DEBUG here (or in the build system) to enabled verbose
1314
// logging using emscripten_dbgf.
@@ -51,7 +52,9 @@ typedef struct thread_profiler_block {
5152
// argument. This can save _emscripten_check_timers from needing to call out to
5253
// JS to get the current time. Passing 0 means that caller doesn't know the
5354
// current time.
54-
void _emscripten_yield(double now);
55+
//
56+
// Returns true is a timer was fired, false otherwise.
57+
bool _emscripten_yield(double now);
5558

5659
void _emscripten_init_main_thread_js(void* tb);
5760
void _emscripten_thread_profiler_enable();

test/codesize/test_codesize_hello_dylink_all.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"a.out.js": 244416,
3-
"a.out.nodebug.wasm": 577664,
4-
"total": 822080,
3+
"a.out.nodebug.wasm": 577672,
4+
"total": 822088,
55
"sent": [
66
"IMG_Init",
77
"IMG_Load",

test/codesize/test_codesize_minimal_pthreads.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
22
"a.out.js": 7323,
33
"a.out.js.gz": 3573,
4-
"a.out.nodebug.wasm": 19095,
5-
"a.out.nodebug.wasm.gz": 8830,
6-
"total": 26418,
7-
"total_gz": 12403,
4+
"a.out.nodebug.wasm": 19065,
5+
"a.out.nodebug.wasm.gz": 8807,
6+
"total": 26388,
7+
"total_gz": 12380,
88
"sent": [
99
"a (memory)",
1010
"b (exit)",

test/codesize/test_codesize_minimal_pthreads_memgrowth.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
22
"a.out.js": 7726,
33
"a.out.js.gz": 3779,
4-
"a.out.nodebug.wasm": 19096,
5-
"a.out.nodebug.wasm.gz": 8831,
6-
"total": 26822,
7-
"total_gz": 12610,
4+
"a.out.nodebug.wasm": 19066,
5+
"a.out.nodebug.wasm.gz": 8808,
6+
"total": 26792,
7+
"total_gz": 12587,
88
"sent": [
99
"a (memory)",
1010
"b (exit)",

0 commit comments

Comments
 (0)