`std::condition_variable::notify_all()` doesn't reliably notify threads resulting in deadlocks

[RobinTF]

Observed Effect

When running a gtest test suite inside WASM on my codebase some tests seem to spuriously hang forever.

Environment

emsdk	6.0.5 (1db513782be24469589d7cb8a1f1834e9a33f271)
Target	wasm64 (-m64), pthreads
Runtime	Node.js v26.5.1, main() on the Node main thread (no PROXY_TO_PTHREAD)
Host	Linux 6.8, x86-64, 32 cores

Analysis of the root cause of the deadlock

Summary

Emscripten arms an Atomics.waitAsync on every pthread's struct and never cancels it. When the thread exits, the struct is freed but the async waiter stays registered on that address for the rest of the process. malloc hands the memory out again. musl wakes a contended lock with Atomics.notify(word, 1), waiters are woken in FIFO order, and the leftover async waiter is ahead of the blocked thread so it absorbs the notify. The notify reports that it woke a waiter, and the thread parked on the word is never released.

The mechanism

src/lib/libpthread.js, _emscripten_thread_mailbox_await:

var wait = Atomics.waitAsync(HEAP32, pthread_ptr / 4, pthread_ptr);
wait.value.then(checkMailbox);

This is how a thread gets told about mailbox messages without blocking. There is no way to cancel an Atomics.waitAsync. The promise only settles on a notify or a timeout, and no timeout is passed here. PThread.returnWorkerToPool frees the struct without ever notifying that address, so every thread that exits leaks exactly one permanently armed waiter.

On the other side, musl's unlock() (in pthread_cond_timedwait.c, which is what a contended std::condition_variable runs through) and pthread_mutex_unlock both wake with a count of 1:

static inline void unlock(volatile int *l) {
  if (a_swap(l, 0) == 2) __wake(l, 1, 1);   // -> Atomics.notify(l, 1)
}

Per spec, Atomics.notify wakes waiters in FIFO order and does not distinguish sync from async ones. The stale async waiter was registered first, so it is the one that gets woken, and it is counted in the return value. The real waiter stays parked forever.

Evidence

Instrumenting emscripten_futex_wait/emscripten_futex_wake with a ring buffer and a 2 s watchdog caught this in the original reproducer:

#54221 tid=694630 wait-enter a=2 b=2   waiter enters wait32(addr, expect 2), memory holds 2
#54222 tid=6cdd50 wake-call  a=1 b=0   unlocker already stored 0, calls notify(addr, 1)
#54225 tid=6cdd50 wake-done  a=1 b=0   notify RETURNED 1 — it claims it woke a waiter
#54247 tid=694630 wait-leave a=2 b=0   that waiter returns 2 = TIMED OUT, 2 s later

The address is not arbitrary. Logging every address passed to Atomics.waitAsync and comparing it with the stuck address:

=== STUCK tid=0x7073e0 addr=0x621710 expected=2 now=0 ===
stuck addr = 0x621710
occurrences in the waitAsync arm log: 1     (out of 62 distinct addresses)

The word the program deadlocks on is the struct of a thread that already exited. In the uninstrumented build it is consistently 0x21690, which is the same address as the lock of my data structure.

Reproducer

emcc -m64 -pthread -O2 -o dl.js RawFutexDeadlock.c && node dl.js

RawFutexDeadlock.c

#include <emscripten.h>
#include <emscripten/threading.h>
#include <pthread.h>
#include <stdatomic.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>

static _Atomic int* word;
static _Atomic int parked = 0;
static _Atomic int woken = 0;

static void* nothing(void* arg) { return NULL; }

static void* waiter(void* arg) {
  atomic_store(&parked, 1);
  // This is what musl's lock() does when it finds the word at 2 ("locked, with
  // waiters"): block until somebody stores something else and notifies.
  __builtin_wasm_memory_atomic_wait32((int*)word, 2, -1 /* no timeout */);
  atomic_store(&woken, 1);
  return NULL;
}

int main(void) {
  // 1. Run a thread to completion.  Its struct is freed, but the async waiter
  //    Emscripten armed on it is still registered.
  pthread_t dead;
  pthread_create(&dead, NULL, nothing, NULL);
  pthread_join(dead, NULL);
  printf("thread ran and exited, its struct was at %p\n", (void*)dead);

  // 2. Get that memory back from malloc and use a word inside it as a lock.
  void* block = malloc(1024);
  if ((uintptr_t)dead < (uintptr_t)block || (uintptr_t)dead >= (uintptr_t)block + 1024) {
    printf("malloc returned %p, which does not cover the old struct -- inconclusive\n", block);
    return 77;
  }
  word = (_Atomic int*)(void*)dead;
  printf("malloc handed back %p; using %p as a lock word\n", block, (void*)word);

  // 3. A thread takes the lock word and blocks on it.
  atomic_store(word, 2);
  pthread_t w;
  pthread_create(&w, NULL, waiter, NULL);
  while (!atomic_load(&parked)) {}
  emscripten_thread_sleep(300);  // let it actually reach the wait

  // 4. Release the lock the way musl's unlock() does.
  atomic_store(word, 0);
  int n = __builtin_wasm_memory_atomic_notify((int*)word, 1);
  printf("released the word and notified 1 waiter; notify reported %d woken\n", n);

  emscripten_thread_sleep(2000);
  if (atomic_load(&woken)) {
    printf("waiter woke up -- no deadlock\n");
    return 0;
  }
  printf("DEADLOCK: notify reported %d woken, but the parked thread is still blocked 2s later\n", n);
  return 1;
}

Fix and workarounds

Release the waiter before the memory is recycled, in PThread.returnWorkerToPool:

Atomics.notify(HEAP32, pthread_ptr / 4);   // add this
__emscripten_thread_free_data(pthread_ptr);

[sbc100]

Interesting that this is wasm64 only. That seems like good clue.

If there is anything you can do to reduce the test case further that would be very useful. Perhaps an AI agent would be good at that?

[RobinTF]

@sbc100 This is unfortunately already the reduced version (using AI). I will try to find out why gtest is necessary for the reproducer and maybe that will give a clue on how to further reduce the reproducer, but this is a job for future me.

[RobinTF]

@sbc100
Great news, AI found a minimum reproducer which even I can understand, and the proposed fix is a one-liner. I've updated the PR description accordingly.

[sbc100]

Wow! Nice work reducing that

[RobinTF]

Full credit to claude, only thing I did is convince it to dig deeper (this ended up being the third attempt overall, the second lead to the initial bug report, but still). There's absolutely no way I would've ever found out that my datastructure lock happens to have the same address as the lock of another thread that has already exited.

[sbc100]

Wow, so in order to reproduce this you need to have reused the pthread struct and then have a lock in that same location?

Do you think we could construct test case that does exactly that?

I think the proposed fix seems reasonable, but I think we might actually want to delay the free'ing of the memory until the thread wakes up. Maybe a thread should be repsonsible for the free'ing its own memory in the waitAsync callback?

[sbc100]

@tlively you might like this one :)

[RobinTF]

@sbc100
This is one of the snippets claude used to identify the mechanism if that helps for your unit tests:

#include <emscripten.h>
#include <pthread.h>
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>

#define NUM_THREADS 8

static void* nothing(void* arg) { return NULL; }

// Number of waiters currently registered on `addr`, as reported by notify.
static int waiters_on(void* addr) {
  return EM_ASM_INT({ return Atomics.notify(HEAP32, $0 / 4, 1); }, (unsigned)(uintptr_t)addr);
}

int main(void) {
  pthread_t dead[NUM_THREADS];
  for (int i = 0; i < NUM_THREADS; ++i) {
    pthread_create(&dead[i], NULL, nothing, NULL);
    pthread_join(dead[i], NULL);
  }

  int leaked = 0;
  for (int i = 0; i < NUM_THREADS; ++i) {
    int n = waiters_on((void*)dead[i]);
    printf("exited thread %2d: struct at %p, waiters still registered: %d\n", i, (void*)dead[i], n);
    leaked += n;
  }
  printf("\n%d of %d exited threads left an uncancellable async waiter behind\n", leaked, NUM_THREADS);

  // Those addresses are free memory now, so malloc will hand them out again.
  int recycled = 0;
  for (int i = 0; i < 64; ++i) {
    void* p = malloc(1024);
    for (int j = 0; j < NUM_THREADS; ++j) {
      if ((uintptr_t)p <= (uintptr_t)dead[j] && (uintptr_t)dead[j] < (uintptr_t)p + 1024) {
        printf("malloc(1024) -> %p covers dead thread %d's struct at %p\n", p, j, (void*)dead[j]);
        ++recycled;
      }
    }
  }
  printf("%d freshly malloc'd blocks contain an address that a stale async waiter is parked on\n", recycled);
  return leaked == 0;
}

[sbc100]

Would you be interested in sending a PR for this?

[sbc100]

I think you could make a more deterministic version that uses a static buffer.

1. Create a static buffer b.
2. Cast b to a pthread and create and then join it
3. Cast b to atomic and do the nofify wait on it.

Then we don't need to rely on the malloc black box to repro maybe?

[RobinTF]

Would you be interested in sending a PR for this?

@sbc100 Usually yes, but I'll be on vacation for the next two weeks, so this will have to wait. (Feel free to fix this before I return though, I'm just glad that I was able to find a plausible reason for the issue)

[sbc100]

Very simply fix for such an tricky issue: #27501