Skip to content

Commit af86ae5

Browse files
committed
Design doc feedback from #26621. NFC
1 parent ecb9e39 commit af86ae5

File tree

2 files changed

+40
-28
lines changed

2 files changed

+40
-28
lines changed

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

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Design Doc: Precise Futex Wakeups in Emscripten
1+
# Design Doc: Precise Futex Wakeups
22

33
- **Status**: Draft
44
- **Bug**: https://github.com/emscripten-core/emscripten/issues/26633
@@ -24,7 +24,7 @@ CPU wakeups and increased latency for events.
2424
## Non-Goals
2525
- **Main Browser Thread**: Changes to the busy-wait loop in `futex_wait_main_browser_thread` are out of scope.
2626
- **Direct Atomics Usage**: Threads that call `atomic.wait` directly (bypassing `emscripten_futex_wait`) will remain un-interruptible.
27-
- **Wasm Workers**: Wasm Worker do not have a `pthread` structure, they are not covered by this design.
27+
- **Wasm Workers**: Wasm Workers do not have a `pthread` structure, so they are not covered by this design.
2828

2929
## Proposed Design
3030

@@ -45,11 +45,11 @@ fields must use `memory_order_seq_cst` to ensure the handshake is robust.
4545
```c
4646
// The address the thread is currently waiting on in emscripten_futex_wait.
4747
// NULL if the thread is not currently in a futex wait.
48-
_Atomic(void*) waiting_on_address;
48+
_Atomic(void*) wait_address;
4949

5050
// A counter that is incremented every time the thread wakes up from a futex wait.
5151
// Used by wakers to ensure the target thread has actually acknowledged the wake.
52-
_Atomic(uint32_t) wait_counter;
52+
_Atomic(uint32_t) wake_counter;
5353

5454
// A bitmask of reasons why the thread was woken for a side-channel event.
5555
_Atomic(uint32_t) wait_reasons;
@@ -62,13 +62,13 @@ _Atomic(uint32_t) wait_reasons;
6262
The waiter will follow this logic (using `SEQ_CST` for all atomic accesses):
6363
6464
1. **Pre-check**: Check `wait_reasons`. If non-zero, handle the reasons (e.g., process mailbox or handle cancellation).
65-
2. **Publish**: Set `waiting_on_address = addr`.
66-
3. **Counter Snapshot**: Read `current_counter = wait_counter`.
67-
4. **Double-check**: This is critical to avoid the race where a reason was added just before `waiting_on_address` was set. If `wait_reasons` is now non-zero, clear `waiting_on_address` and go to step 1.
65+
2. **Publish**: Set `wait_address = addr`.
66+
3. **Counter Snapshot**: Read `current_counter = wake_counter`.
67+
4. **Double-check**: This is critical to avoid the race where a reason was added just before `wait_address` was set. If `wait_reasons` is now non-zero, clear `wait_address` and go to step 1.
6868
5. **Wait**: Call `ret = __builtin_wasm_memory_atomic_wait32(addr, val, timeout)`.
6969
6. **Unpublish**:
70-
- Set `waiting_on_address = NULL`.
71-
- Atomically increment `wait_counter`.
70+
- Set `wait_address = NULL`.
71+
- Atomically increment `wake_counter`.
7272
7. **Post-check**: Check `wait_reasons`. If non-zero, handle the reasons.
7373
8. **Return**: Return the result of the wait to the caller.
7474
- If `ret == ATOMICS_WAIT_OK`, return `0`.
@@ -81,18 +81,18 @@ Note: We do **not** loop internally if `ret == ATOMICS_WAIT_OK`. Even if we susp
8181
When a thread needs to wake another thread for a side-channel event (e.g., in `pthread_cancel` or `em_task_queue_enqueue`):
8282
8383
1. Atomically OR the appropriate bit into the target thread's `wait_reasons` (`SEQ_CST`).
84-
2. Read `target_addr = target->waiting_on_address` (`SEQ_CST`).
84+
2. Read `target_addr = target->wait_address` (`SEQ_CST`).
8585
3. If `target_addr` is not NULL:
86-
- Read `start_c = target->wait_counter` (`SEQ_CST`).
86+
- Read `start_c = target->wake_counter` (`SEQ_CST`).
8787
- Enter a loop:
88-
- Call `emscripten_futex_wake(target_addr, 1)`.
89-
- Exit loop if `target->wait_counter != start_c` OR `target->waiting_on_address != target_addr`.
88+
- Call `emscripten_futex_wake(target_addr, INT_MAX)`.
89+
- Exit loop if `target->wake_counter != start_c` OR `target->wait_address != target_addr`.
9090
- **Yield**: Call `sched_yield()` (or a small sleep) to allow the target thread to proceed if it is currently being scheduled.
9191
9292
### 4. Handling the Race Condition
9393
The "Lost Wakeup" race is handled by the combination of:
94-
- The waiter double-checking `wait_reasons` after publishing its `waiting_on_address`.
95-
- The waker looping `atomic.wake` until the waiter increments its `wait_counter`.
94+
- The waiter double-checking `wait_reasons` after publishing its `wait_address`.
95+
- The waker looping `atomic.wake` until the waiter increments its `wake_counter`.
9696
9797
Even if the waker's first `atomic.wake` occurs after the waiter's double-check
9898
but *before* the waiter actually enters the `atomic.wait` instruction, the waker
@@ -106,19 +106,20 @@ counter.
106106
### 5. Overlapping and Spurious Wakeups
107107
The design must handle cases where "real" wakeups (triggered by the application) and "side-channel" wakeups (cancellation/mailbox) occur simultaneously.
108108
109-
1. **Spurious Wakeups for Other Threads**: If multiple threads are waiting on the same address (e.g., a shared mutex), a side-channel `atomic_wake(addr, 1)` targeted at Thread A might be delivered by the kernel to Thread B.
110-
- **Thread B's response**: It will wake up, increment its `wait_counter`, see that its `wait_reasons` are empty, and return `0` to its caller.
111-
- **Thread C (the waker)**: It will see that Thread A's `wait_counter` has *not* changed and `waiting_on_address` is still `addr`. It will therefore continue its loop and call `atomic_wake` again until Thread A is finally woken.
109+
1. **Spurious Wakeups for Other Threads**: If multiple threads are waiting on the same address (e.g., a shared mutex), a side-channel `atomic_wake(addr, 1)` targeting Thread A could also wake Thread B.
110+
- **Thread B's response**: It will wake up, increment its `wake_counter`, see that its `wait_reasons` are empty, and return `0` to its caller.
111+
- **Thread C (the waker)**: It will see that Thread A's `wake_counter` has *not* changed and `wait_address` is still `addr`. It will therefore continue its loop and call `atomic_wake` again until Thread A is finally woken.
112112
- **Result**: Thread B experiences a "spurious" wakeup. This is acceptable and expected behavior for futex-based synchronization.
113113
2. **Handling Side-Channel Success**: If Thread A is woken by the side-channel, it handles the event and returns `0`. The user's code will typically see that its own synchronization condition is not yet met and immediately call `emscripten_futex_wait` again. This effectively "resumes" the wait from the user's perspective while having allowed the side-channel event to be processed.
114114
3. **No Lost "Real" Wakeups**: By returning to the caller whenever `atomic.wait` returns `OK`, we ensure that we never miss or swallow a real application-level `atomic.wake`.
115115
116116
### 6. Counter Wrap-around
117-
The `wait_counter` is a `uint32_t` and will wrap around to zero after $2^{32}$ wakeups. This is safe because:
118-
1. **Impossibility of Racing**: For the waker to "miss" a wake-up due to wrap-around, the waiter would have to wake up and re-enter a sleep state exactly $2^{32}$ times in the very brief window between the waker's `atomic_wake` and its subsequent check of `wait_counter`. Even at extreme wakeup frequencies (e.g., 1 million per second), this would take over an hour.
119-
2. **Address Change Check**: The waker loop also checks `target->waiting_on_address != target_addr`. If the waiter wakes up and either stops waiting or starts waiting on a *different* address, the waker will exit the loop regardless of the counter value.
117+
The `wake_counter` is a `uint32_t` and will wrap around to zero after $2^{32}$ wakeups. This is safe because:
118+
1. **Impossibility of Racing**: For the waker to "miss" a wake-up due to wrap-around, the waiter would have to wake up and re-enter a sleep state exactly $2^{32}$ times in the very brief window between the waker's `atomic_wake` and its subsequent check of `wake_counter`. Even at extreme wakeup frequencies (e.g., 1 million per second), this would take over an hour.
119+
2. **Address Change Check**: The waker loop also checks `target->wait_address != target_addr`. If the waiter wakes up and either stops waiting or starts waiting on a *different* address, the waker will exit the loop regardless of the counter value.
120+
121+
## Benefits
120122
121-
### 6. Benefits
122123
- **Lower Power Consumption**: Threads can sleep indefinitely (or for the full duration of a user-requested timeout) without periodic wakeups.
123124
- **Lower Latency**: Mailbox events and cancellation requests are processed immediately rather than waiting for the next 1ms or 100ms tick.
124125
- **Simpler Loop**: The complex logic for calculating remaining timeout slices in `emscripten_futex_wait` is removed.
@@ -132,8 +133,8 @@ The `wait_counter` is a `uint32_t` and will wrap around to zero after $2^{32}$ w
132133
design works around this by having the waker use the *user's* futex address.
133134
134135
## Security/Safety Considerations
135-
- The `waiting_on_address` must be managed carefully to ensure wakers don't
136-
call `atomic.wake` on stale addresses. The `wait_counter` and clearing the
136+
- The `wait_address` must be managed carefully to ensure wakers don't
137+
call `atomic.wake` on stale addresses. The `wake_counter` and clearing the
137138
address upon wake mitigate this.
138139
- The waker loop should have a reasonable fallback (like a yield) to prevent a
139140
busy-wait deadlock if the waiter is somehow prevented from waking up (though

docs/design/README.md

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,19 @@
33
This directory contains design documents for emscripten features and major
44
changes/refactors.
55

6-
We are experimenting with keeping these document here under source control with
6+
We are experimenting with keeping these documents here under source control with
77
the hope that this will increase understandability of the codebase. This has
8-
some advantages over doing all planning in Google Docs or GitHub issues. For
9-
example, it allows us to track the history of designs and it allows them to be
10-
searchable using standard tools like `git grep`.
8+
some advantages over doing all our planning in Google Docs or GitHub issues.
9+
For example, it allows us to track the history of designs and it allows them to
10+
be searchable using standard tools like `git grep`.
11+
12+
## Document Format
13+
14+
Each document in this directory should be a markdown file. At the top of each
15+
document should be a `Status` which can be either `Draft`, `Accepted`,
16+
`Completed`.
17+
18+
When a document is marked as `Completed` it should also be updated such that
19+
it is clear the work has been done, and is now in the past. For example,
20+
phrases such as "The current behavior" should be replaced with "The previous
21+
behavior".

0 commit comments

Comments
 (0)