Implement precise pthread wakeups

ChangeLog.md

@@ -20,6 +20,8 @@ See docs/process.md for more on how version tagging works.
 
 5.0.6 (in development)
 ----------------------
+- The emscripten_futux_wait API is not document to explicitly allow spurious
+  wakeups. (#26659)
 - The minimum version of node supported by the generated code was bumped from
   v12.22.0 to v18.3.0. (#26604)
 - The DETERMINISIC settings was marked as deprecated (#26653)

docs/design/01-precise-futex-wakeups.md

@@ -1,18 +1,18 @@
 # Design Doc: Precise Futex Wakeups
 
-- **Status**: Draft
+- **Status**: Completed
 - **Bug**: https://github.com/emscripten-core/emscripten/issues/26633
 
 ## Context
-Currently, `emscripten_futex_wait` (in
-`system/lib/pthread/emscripten_futex_wait.c`) relies on a periodic wakeup loop
-for pthreads and the main runtime thread. This is done for two primary reasons:
+Historically, `emscripten_futex_wait` (in
+`system/lib/pthread/emscripten_futex_wait.c`) relied on a periodic wakeup loop
+for pthreads and the main runtime thread. This was done for two primary reasons:
 
-1. **Thread Cancellation**: To check if the calling thread has been cancelled while it is blocked.
+1. **Thread Cancellation**: To check if the calling thread had been cancelled while it was blocked.
 2. **Main Runtime Thread Events**: To allow the main runtime thread (even when not the main browser thread) to process its mailbox/event queue.
 
-The current implementation uses a 1ms wakeup interval for the main runtime
-thread and a 100ms interval for cancellable pthreads. This leads to unnecessary
+The old implementation used a 1ms wakeup interval for the main runtime
+thread and a 100ms interval for cancellable pthreads. This led to unnecessary
 CPU wakeups and increased latency for events.
 
 ## Goals
@@ -23,22 +23,21 @@ CPU wakeups and increased latency for events.
 
 ## Non-Goals
 - **Main Browser Thread**: Changes to the busy-wait loop in `futex_wait_main_browser_thread` are out of scope.
-- **Direct Atomics Usage**: Threads that call `atomic.wait` directly (bypassing `emscripten_futex_wait`) will remain un-interruptible.
+- **Direct Atomics Usage**: Threads that call `atomic.wait` directly (bypassing `emscripten_futex_wait`) remain un-interruptible.
 - **Wasm Workers**: Wasm Workers do not have a `pthread` structure, so they are not covered by this design.
 
-## Proposed Design
+## Design
 
 The core idea is to allow "side-channel" wakeups (cancellation, mailbox events)
 to interrupt the `atomic.wait` call by having the waker call `atomic.wake` on the
 same address the waiter is currently blocked on.
 
-As part of this design we will need to explicitly state that
-`emscripten_futex_wait` now supports spurious wakeups.  i.e. it may return `0`
-(success) even if the underlying futex was not explicitly woken by the
-application.
+As part of this design, `emscripten_futex_wait` now explicitly supports spurious
+wakeups.  i.e. it may return `0` (success) even if the underlying futex was not
+explicitly woken by the application.
 
 ### 1. `struct pthread` Extensions
-We will add a single atomic `wait_addr` field to `struct pthread` (in
+A single atomic `wait_addr` field was added to `struct pthread` (in
 `system/lib/libc/musl/src/internal/pthread_impl.h`).
 
 ```c
@@ -57,82 +56,80 @@ _Atomic uintptr_t wait_addr;
 ```
 
 ### 2. Waiter Logic (`emscripten_futex_wait`)
-The waiter will follow this logic:
+The waiter follows this logic:
 
-1.  **Notification Loop**:
+1.  **Publish Wait Address**:
     ```c
     uintptr_t expected_null = 0;
-    while (!atomic_compare_exchange_strong(&self->wait_addr, &expected_null, (uintptr_t)addr)) {
+    if (!atomic_compare_exchange_strong(&self->wait_addr, &expected_null, (uintptr_t)addr)) {
         // If the CAS failed, it means NOTIFY_BIT was set by another thread.
-        assert(expected_null == NOTIFY_BIT);
-        // Let the notifier know that we received the wakeup notification by
-        // resetting wait_addr.
-        self->wait_addr = 0;
-        handle_wakeup(); // Process mailbox or handle cancellation
-        // Reset expected_null because CAS updates it to the observed value on failure.
-        expected_null = 0;
+        assert(expected_null & NOTIFY_BIT);
+        // We don't wait at all; instead behave as if we spuriously woke up.
+        ret = ATOMICS_WAIT_OK;
+        goto done;
     }
     ```
 2.  **Wait**: Call `ret = __builtin_wasm_memory_atomic_wait32(addr, val, timeout)`.
-3.  **Unpublish & Check**:
+3.  **Unpublish**:
     ```c
-    // Clear wait_addr and check if a notification arrived while we were sleeping.
-    if ((atomic_exchange(&self->wait_addr, 0) & NOTIFY_BIT) != 0) {
-        handle_wakeup();
-    }
+    done:
+    self->wait_addr = 0;
     ```
-4.  **Return**: Return the result of the wait.
+4.  **Handle side effects**: If the wake was due to cancellation or mailbox
+    events, these are handled after `emscripten_futex_wait` returns (or
+    internally via `pthread_testcancel` if cancellable).
 
 Note: We do **not** loop internally if `ret == ATOMICS_WAIT_OK`. Even if we
 suspect the wake was caused by a side-channel event, we must return to the user
 to avoid "swallowing" a simultaneous real application wake.
 
-### 3. Waker Logic
-When a thread needs to wake another thread for a side-channel event:
+### 3. Waker Logic (`_emscripten_thread_notify`)
+When a thread needs to wake another thread for a side-channel event (e.g.
+enqueuing work or cancellation), it calls `_emscripten_thread_notify`:
 
-1.  **Enqueue Work**: Add the task to the target's mailbox or set the cancellation flag.
-2.  **Signal**:
-    ```c
-    uintptr_t addr = atomic_fetch_or(&target->wait_addr, NOTIFY_BIT);
-    if (addr == 0 || (addr & NOTIFY_BIT) != 0) {
-        // Either the thread wasn't waiting (it will see NOTIFY_BIT later),
-        // or someone else is already in the process of notifying it.
-        return;
-    }
-    // We set the bit and are responsible for waking the target.
-    // The target is currently waiting on `addr`.
-    while (target->wait_addr == (addr | NOTIFY_BIT)) {
-        emscripten_futex_wake((void*)addr, INT_MAX);
-        sched_yield();
-    }
-    ```
+```c
+void _emscripten_thread_notify(pthread_t target) {
+  uintptr_t addr = atomic_fetch_or(&target->wait_addr, NOTIFY_BIT);
+  if (addr == 0 || (addr & NOTIFY_BIT) != 0) {
+    // Either the thread wasn't waiting (it will see NOTIFY_BIT later),
+    // or someone else is already in the process of notifying it.
+    return;
+  }
+  // We set the bit and are responsible for waking the target.
+  // The target is currently waiting on `addr`.
+  while (target->wait_addr == (addr | NOTIFY_BIT)) {
+    emscripten_futex_wake((void*)addr, INT_MAX);
+    sched_yield();
+  }
+}
+```
 
 ### 4. Handling the Race Condition
 The protocol handles the "Lost Wakeup" race by having the waker loop until the
 waiter clears its `wait_addr`. If the waker sets the `NOTIFY_BIT` just before
 the waiter enters `atomic.wait`, the `atomic_wake` will be delivered once the
 waiter is asleep. If the waiter wakes up for any reason (timeout, real wake, or
-side-channel wake), its `atomic_exchange` will satisfy the waker's loop
-condition.
+side-channel wake), its reset of `wait_addr` to `0` will satisfy the waker's
+loop condition.
 
 ## Benefits
 
 - **Lower Power Consumption**: Threads can sleep indefinitely (or for the full duration of a user-requested timeout) without periodic wakeups.
-- **Lower Latency**: Mailbox events and cancellation requests are processed immediately rather than waiting for the next 1ms or 100ms tick.
-- **Simpler Loop**: The complex logic for calculating remaining timeout slices in `emscripten_futex_wait` is removed.
+- **Lower Latency**: Mailbox events and cancellation requests are processed immediately rather than waiting for the next tick.
+- **Simpler Loop**: The complex logic for calculating remaining timeout slices in `emscripten_futex_wait` was removed.
 
 ## Alternatives Considered
 - **Signal-based wakeups**: Not currently feasible in Wasm as signals are not
   implemented in a way that can interrupt `atomic.wait`.
 - **A single global "wake-up" address per thread**: This would require the
   waiter to wait on *two* addresses simultaneously (the user's futex and its
-  own wakeup address), which `atomic.wait` does not support. The proposed
+  own wakeup address), which `atomic.wait` does not support. The implemented
   design works around this by having the waker use the *user's* futex address.
 
 ## Security/Safety Considerations
-- **The `wait_addr` must be managed carefully** to ensure wakers don't
+- **The `wait_addr` is managed carefully** to ensure wakers don't
   call `atomic.wake` on stale addresses. Clearing the address upon wake
   mitigates this.
-- **The waker loop should have a reasonable fallback** (like a yield) to prevent a
-  busy-wait deadlock if the waiter is somehow prevented from waking up (though
-  `atomic.wait` is generally guaranteed to wake if `atomic.wake` is called).
+- **The waker loop has a yield** to prevent a busy-wait deadlock if the waiter
+  is somehow prevented from waking up (though `atomic.wait` is generally
+  guaranteed to wake if `atomic.wake` is called).

src/struct_info_generated.json

@@ -1036,7 +1036,7 @@
             "p_proto": 8
         },
         "pthread": {
-            "__size__": 124,
+            "__size__": 128,
             "profilerBlock": 104,
             "stack": 48,
             "stack_size": 52,

src/struct_info_generated_wasm64.json

@@ -1036,7 +1036,7 @@
             "p_proto": 16
         },
         "pthread": {
-            "__size__": 216,
+            "__size__": 224,
             "profilerBlock": 184,
             "stack": 80,
             "stack_size": 88,

system/include/emscripten/threading_primitives.h

@@ -188,6 +188,10 @@ void emscripten_condvar_signal(emscripten_condvar_t * _Nonnull condvar, uint32_t
 
 // If the given memory address contains value val, puts the calling thread to
 // sleep waiting for that address to be notified.
+// Note: Like the Linux futex syscall, this APi *does* allow spurious wakeups.
+// This differs from the WebAssembly `atomic.wait` instruction itself which
+// does *not* allow supurious wakeups and it means that most callers will want
+// to wraps this some kind loop.
 // Returns -EINVAL if addr is null.
 int emscripten_futex_wait(volatile void/*uint32_t*/ * _Nonnull addr, uint32_t val, double maxWaitMilliseconds);
 

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

@@ -111,6 +111,16 @@ struct pthread {
 	// postMessage path. Once this becomes true, it remains true so we never
 	// fall back to postMessage unnecessarily.
 	_Atomic int waiting_async;
+	// The address the thread is currently waiting on in emscripten_futex_wait.
+	//
+	// This field encodes the state using the following bitmask:
+	// - NULL: Not waiting, no pending notification.
+	// - NOTIFY_BIT (0x1): Not waiting, but a notification was sent.
+	// - addr: Waiting on `addr`, no pending notification.
+	// - addr | NOTIFY_BIT: Waiting on `addr`, notification sent.
+	//
+	// Since futex addresses must be 4-byte aligned, the low bit is safe to use.
+	_Atomic uintptr_t wait_addr;
 #endif
 #ifdef EMSCRIPTEN_DYNAMIC_LINKING
 	// When dynamic linking is enabled, threads use this to facilitate the
@@ -120,6 +130,10 @@ struct pthread {
 #endif
 };
 
+#ifdef __EMSCRIPTEN__
+#define NOTIFY_BIT (1 << 0)
+#endif
+
 enum {
 	DT_EXITED,
 	DT_EXITING,

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

@@ -8,6 +8,7 @@
 #include <emscripten/emscripten.h>
 #include <emscripten/threading.h>
 #include <assert.h>
+#include <math.h>
 #include <signal.h>
 #include <stdint.h>
 #include <stdio.h>
@@ -79,6 +80,18 @@ void _emscripten_check_timers(double now)
 		}
 	}
 }
+
+double _emscripten_next_timer()
+{
+	assert(emscripten_is_main_runtime_thread());
+	double next_timer = INFINITY;
+	for (int which = 0; which < 3; which++) {
+		if (current_timeout_ms[which]) {
+			next_timer = fmin(current_timeout_ms[which], next_timer);
+		}
+	}
+	return next_timer - emscripten_get_now();
+}
 #endif
 
 int setitimer(int which, const struct itimerval *restrict new, struct itimerval *restrict old)

system/lib/libc/musl/src/thread/pthread_cancel.c

@@ -108,5 +108,13 @@ int pthread_cancel(pthread_t t)
 			pthread_exit(PTHREAD_CANCELED);
 		return 0;
 	}
+#ifdef __EMSCRIPTEN__
+	// Wake the target thread in case it in emscripten_futex_wait.  Normally
+	// this is only done when the target is the main runtime thread and there
+	// is an event added to its system queue.
+	// However, all threads needs to be inturruped like this in the case they are
+	// cancelled.
+	_emscripten_thread_notify(t);
+#endif
 	return pthread_kill(t, SIGCANCEL);
 }

system/lib/pthread/em_task_queue.c

@@ -14,6 +14,7 @@
 #include "em_task_queue.h"
 #include "proxying_notification_state.h"
 #include "thread_mailbox.h"
+#include "threading_internal.h"
 
 #define EM_TASK_QUEUE_INITIAL_CAPACITY 128
 
@@ -166,6 +167,7 @@ static bool em_task_queue_grow(em_task_queue* queue) {
 }
 
 void em_task_queue_execute(em_task_queue* queue) {
+  DBG("em_task_queue_execute");
   queue->processing = 1;
   pthread_mutex_lock(&queue->mutex);
   while (!em_task_queue_is_empty(queue)) {
@@ -178,6 +180,7 @@ void em_task_queue_execute(em_task_queue* queue) {
   }
   pthread_mutex_unlock(&queue->mutex);
   queue->processing = 0;
+  DBG("done em_task_queue_execute");
 }
 
 void em_task_queue_cancel(em_task_queue* queue) {
@@ -219,6 +222,7 @@ static void receive_notification(void* arg) {
   notification_state expected = NOTIFICATION_RECEIVED;
   atomic_compare_exchange_strong(
     &tasks->notification, &expected, NOTIFICATION_NONE);
+  DBG("receive_notification done");
 }
 
 static void cancel_notification(void* arg) {
@@ -246,6 +250,7 @@ bool em_task_queue_send(em_task_queue* queue, task t) {
   notification_state previous =
     atomic_exchange(&queue->notification, NOTIFICATION_PENDING);
   if (previous == NOTIFICATION_PENDING) {
+    DBG("em_task_queue_send NOTIFICATION_PENDING already set");
     emscripten_thread_mailbox_unref(queue->thread);
     return true;
   }