assertion failure when calling `terminateAllThreads` as a thread is finishing

[bakkot]

This is with emcc version

emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.0.2-git
clang version 14.0.0 (https://github.com/llvm/llvm-project.git 1a929525e86a20d0b3455a400d0dbed40b325a13)
Target: wasm32-unknown-emscripten
Thread model: posix
InstalledDir: /usr/local/opt/emscripten/libexec/llvm/bin

Here's some code:

// lib.cc
#include <stdio.h>

#include <thread>

#include <emscripten.h>


int do_work() {
  return 42;
}

extern "C" void async_do_work() {
  std::thread t([&] {
    int result = do_work();
    MAIN_THREAD_ASYNC_EM_ASM({
      resolve($0);
    }, result);
  });
  t.detach();
}

and a wrapper, injected with --pre-js:

// wrapper.js
let resolve;

Module.do_work = () => {
  let promise = new Promise(res => { resolve = res; });
  Module._async_do_work();
  return promise;
};

Compiled with

emcc -std=c++17 lib.cc -s USE_PTHREADS=1 -s EXPORTED_FUNCTIONS=_async_do_work -s DEMANGLE_SUPPORT=1 -s MODULARIZE=1 -s EXPORT_NAME=Init --pre-js wrapper.js -o lib.js

and run with

// main.js
'use strict';

let init = typeof require === 'function' ? require('./lib.js') : Init;

(async () => {
  let mod = await init();
  console.log(await mod.do_work());

  // await new Promise(res => setTimeout(res, 0));

  console.log('cleaning up...');
  mod.PThread.terminateAllThreads();
})().catch(e => {
  console.log('caught');
  console.error(e);
  process.exit(1);
});

this produces

42
cleaning up...
Aborted(Assertion failed)
/Users/kevin/Code/emscripten-call-on-thread-bug/lib.js:235
      throw ex;
      ^

RuntimeError: Aborted(Assertion failed)
    at abort (/Users/kevin/Code/emscripten-call-on-thread-bug/lib.js:1745:11)
    at assert (/Users/kevin/Code/emscripten-call-on-thread-bug/lib.js:874:5)
    at Worker.worker.onmessage (/Users/kevin/Code/emscripten-call-on-thread-bug/lib.js:2233:13)
    at Worker.<anonymous> (/Users/kevin/Code/emscripten-call-on-thread-bug/lib.js:2256:20)
    at Worker.emit (node:events:390:28)
    at MessagePort.<anonymous> (node:internal/worker:232:53)
    at MessagePort.[nodejs.internal.kHybridDispatch] (node:internal/event_target:562:20)
    at MessagePort.exports.emitMessage (node:internal/per_context/messageport:23:28)

The relevant lines in lib.js are

} else if (cmd === 'detachedExit') {
  assert(worker.pthread);
  PThread.returnWorkerToPool(worker);
}

If you un-comment the await new Promise line in main.js it will work - it looks like the problem is specifically calling terminateAllThreads while a thread is in the process of shutting down already. Fortunately waiting until the next turn of the event loop is a straightforward workaround, so it's probably not a big deal.

#9486 had a similar exception, but it's not the same one.

[sbc100]

I'm not sure it will make any different to the assertion, but calling PThread.terminateAllThreads() is probably no a good idea. Instead its probably better to simply call mod._exit (and add _exit to your list of exported functions).

I seems like there must be a race between terminateAllThreads and the detached exit... and I imagine this will effect any application that tries to exit with detached threads. One problem is that terminateAllThreads is a very blunt instrument which is inherently racing with the execution of the threads. There are a couple of improvements I'd like to make to avoid this kind of issue:

1. Attempt to bring down thread cooperatively before terminating them (i.e. given them a chance to cancelled).
2. Set some global state before calling terminateAllThreads that will cause all future message from all threads to be ignored as spurious.