Fix pthread_kill behaviour

All `pthread_kill` really does is ensure that the signal handler for
a given signal occurs on a specific thread.

The slight exception to this is SIGCANCEL which is how pthread_cancel is
implemented internally.

This allows us to completely remove the cancelThread and killThread
JS functions.

Fixes: emscripten-core#22274

src/library.js

@@ -2021,7 +2021,6 @@ addToLibrary({
     throw 'unwind';
   },
 
-  _emscripten_runtime_keepalive_clear__proxy: 'sync',
   _emscripten_runtime_keepalive_clear: () => {
 #if isSymbolNeeded('$noExitRuntime')
     noExitRuntime = false;

src/library_pthread.js

@@ -34,9 +34,8 @@ globalThis.MAX_PTR = (2 ** 32) - 1
 var LibraryPThread = {
   $PThread__postset: 'PThread.init();',
   $PThread__deps: ['_emscripten_thread_init',
-                   '$killThread',
                    '$terminateWorker',
-                   '$cancelThread', '$cleanupThread', '$zeroMemory',
+                   '$cleanupThread', '$zeroMemory',
 #if MAIN_MODULE
                    '$markAsFinished',
 #endif
@@ -273,10 +272,6 @@ var LibraryPThread = {
         } else if (cmd === 'markAsFinished') {
           markAsFinished(d['thread']);
 #endif
-        } else if (cmd === 'killThread') {
-          killThread(d['thread']);
-        } else if (cmd === 'cancelThread') {
-          cancelThread(d['thread']);
         } else if (cmd === 'loaded') {
           worker.loaded = true;
 #if ENVIRONMENT_MAY_BE_NODE && PTHREAD_POOL_SIZE
@@ -533,25 +528,6 @@ var LibraryPThread = {
     };
   },
 
-  $killThread__deps: ['_emscripten_thread_free_data', '$terminateWorker'],
-  $killThread: (pthread_ptr) => {
-#if PTHREADS_DEBUG
-    dbg(`killThread ${ptrToString(pthread_ptr)}`);
-#endif
-#if ASSERTIONS
-    assert(!ENVIRONMENT_IS_PTHREAD, 'Internal Error! killThread() can only ever be called from main application thread!');
-    assert(pthread_ptr, 'Internal Error! Null pthread_ptr in killThread!');
-#endif
-    var worker = PThread.pthreads[pthread_ptr];
-    delete PThread.pthreads[pthread_ptr];
-    terminateWorker(worker);
-    __emscripten_thread_free_data(pthread_ptr);
-    // The worker was completely nuked (not just the pthread execution it was hosting), so remove it from running workers
-    // but don't put it back to the pool.
-    PThread.runningWorkers.splice(PThread.runningWorkers.indexOf(worker), 1); // Not a running Worker anymore.
-    worker.pthread_ptr = 0;
-  },
-
   _emscripten_thread_cleanup: (thread) => {
     // Called when a thread needs to be cleaned up so it can be reused.
     // A thread is considered reusable when it either returns from its
@@ -639,15 +615,6 @@ var LibraryPThread = {
   $registerTLSInit: (tlsInitFunc) => PThread.tlsInitFunctions.push(tlsInitFunc),
 #endif
 
-  $cancelThread: (pthread_ptr) => {
-#if ASSERTIONS
-    assert(!ENVIRONMENT_IS_PTHREAD, 'Internal Error! cancelThread() can only ever be called from main application thread!');
-    assert(pthread_ptr, 'Internal Error! Null pthread_ptr in cancelThread!');
-#endif
-    var worker = PThread.pthreads[pthread_ptr];
-    worker.postMessage({ 'cmd': 'cancel' });
-  },
-
   $spawnThread: (threadParams) => {
 #if ASSERTIONS
     assert(!ENVIRONMENT_IS_PTHREAD, 'Internal Error! spawnThread() can only ever be called from main application thread!');
@@ -918,17 +885,6 @@ var LibraryPThread = {
 #endif
   },
 
-  __pthread_kill_js: (thread, signal) => {
-    if (signal === {{{ cDefs.SIGCANCEL }}}) { // Used by pthread_cancel in musl
-      if (!ENVIRONMENT_IS_PTHREAD) cancelThread(thread);
-      else postMessage({ 'cmd': 'cancelThread', 'thread': thread });
-    } else {
-      if (!ENVIRONMENT_IS_PTHREAD) killThread(thread);
-      else postMessage({ 'cmd': 'killThread', 'thread': thread });
-    }
-    return 0;
-  },
-
   // This function is call by a pthread to signal that exit() was called and
   // that the entire process should exit.
   // This function is always called from a pthread, but is executed on the

src/library_sigs.js

@@ -226,7 +226,6 @@ sigs = {
   __cxa_uncaught_exceptions__sig: 'i',
   __handle_stack_overflow__sig: 'vp',
   __pthread_create_js__sig: 'ipppp',
-  __pthread_kill_js__sig: 'ipi',
   __resumeException__sig: 'vp',
   __syscall__newselect__sig: 'iipppp',
   __syscall_accept4__sig: 'iippiii',

src/runtime_pthread.js

@@ -222,10 +222,6 @@ if (ENVIRONMENT_IS_PTHREAD) {
           dbg(`worker: Pthread 0x${_pthread_self().toString(16)} completed its main entry point with an 'unwind', keeping the worker alive for asynchronous operation.`);
 #endif
         }
-      } else if (cmd === 'cancel') { // Main thread is asking for a pthread_cancel() on this thread.
-        if (_pthread_self()) {
-          __emscripten_thread_exit({{{ cDefs.PTHREAD_CANCELED }}});
-        }
       } else if (msgData.target === 'setimmediate') {
         // no-op
       } else if (cmd === 'checkMailbox') {

system/lib/pthread/pthread_kill.c

@@ -5,11 +5,25 @@
  * found in the LICENSE file.
  */
 
-#include <emscripten/threading.h>
+#include <threading_internal.h>
+#include <emscripten/proxying.h>
+#include <emscripten/console.h>
+#include <emscripten_internal.h>
 
 #include "pthread_impl.h"
 #include "lock.h"
 
+void do_raise(void* arg) {
+  int sig = (intptr_t)arg;
+  // For SIGCANCEL is simply enough to run the proxied function since
+  // we call pthread_testcancel after running each queued function.
+  if (sig == SIGCANCEL) {
+    _emscripten_runtime_keepalive_clear();
+    return;
+  }
+  raise((intptr_t)sig);
+}
+
 int pthread_kill(pthread_t t, int sig) {
   if (sig < 0 || sig >= _NSIG) {
     return EINVAL;
@@ -22,5 +36,7 @@ int pthread_kill(pthread_t t, int sig) {
     return ESRCH;
   }
   if (sig == 0) return 0; // signal == 0 is a no-op.
-  return __pthread_kill_js(t, sig);
+  //emscripten_errf("pthread_kill %d", sig);
+  emscripten_proxy_async(emscripten_proxy_get_system_queue(), t, do_raise, (void*)sig);
+  return 0;
 }

system/lib/pthread/thread_mailbox.c

@@ -84,6 +84,9 @@ void _emscripten_check_mailbox() {
   notification_state expected = NOTIFICATION_RECEIVED;
   atomic_compare_exchange_strong(
     &mailbox->notification, &expected, NOTIFICATION_NONE);
+  // After every mailbox check we call `__pthread_testcancel` in case
+  // one of the proxied functions was from pthread_kill(SIGCANCEL).
+  __pthread_testcancel();
 }
 
 void emscripten_thread_mailbox_send(pthread_t thread, task t) {

system/lib/pthread/threading_internal.h

@@ -66,6 +66,7 @@ void _emscripten_thread_set_strongref(pthread_t thread);
 int _emscripten_thread_is_valid(pthread_t thread);
 
 void _emscripten_thread_exit_joinable(pthread_t thread);
+void _emscripten_thread_exit(void* result);
 void _emscripten_process_dlopen_queue(void);
 
 #ifdef NDEBUG

test/other/codesize/test_codesize_minimal_pthreads.gzsize

@@ -1 +1 @@
-5100
+5018

test/other/codesize/test_codesize_minimal_pthreads.jssize

@@ -1 +1 @@
-11035
+10845

test/pthread/test_pthread_kill.c

@@ -13,52 +13,58 @@
 #include <signal.h>
 
 _Atomic int sharedVar = 0;
+_Atomic int gotTermSignal = 0;
 
-static void *thread_start(void *arg) {
-  // As long as this thread is running, keep the shared variable latched to nonzero value.
-  for (;;) {
-    ++sharedVar;
+pthread_t thr;
+
+void signal_handler(int sig, siginfo_t * info, void * arg) {
+  printf("signal: %d onthread=%d\n", sig, pthread_self() == thr);
+  if (sig == SIGTERM) {
+    gotTermSignal = 1;
   }
+}
 
-  pthread_exit(0);
+void setup_handler() {
+  struct sigaction act;
+  sigemptyset(&act.sa_mask);
+  act.sa_flags = SA_SIGINFO;
+  act.sa_sigaction = signal_handler;
+  sigaction(SIGTERM, &act, NULL);
 }
 
-void BusySleep(long msecs) {
+
+void sleepms(long msecs) {
   usleep(msecs * 1000);
 }
 
+void *thread_start(void *arg) {
+  // As long as this thread is running, keep the shared variable latched to nonzero value.
+  while (!gotTermSignal) {
+    sleepms(1);
+    ++sharedVar;
+  }
+  printf("got term signal, shutting down thread\n");
+  pthread_exit(0);
+}
+
 int main() {
-  pthread_t thr;
+  setup_handler();
 
   sharedVar = 0;
   int s = pthread_create(&thr, NULL, thread_start, 0);
   assert(s == 0);
 
   // Wait until thread kicks in and sets the shared variable.
-  while (sharedVar == 0)
-    BusySleep(10);
-  printf("thread startd\n");
-
-  s = pthread_kill(thr, SIGKILL);
-  printf("thread killed\n");
-  assert(s == 0);
-
-  // Wait until we see the shared variable stop incrementing. (This is a bit heuristic and hacky)
-  for(;;) {
-    int val = sharedVar;
-    BusySleep(100);
-    int val2 = sharedVar;
-    if (val == val2) break;
+  while (sharedVar == 0) {
+    sleepms(10);
   }
+  printf("thread has started, sending SIGTERM\n");
 
-  // Reset to 0.
-  sharedVar = 0;
+  s = pthread_kill(thr, SIGTERM);
+  printf("SIGTERM sent\n");
 
-  // Wait for a long time, if the thread is still running, it should progress and set sharedVar by this time.
-  BusySleep(1000);
+  assert(s == 0);
 
-  // Finally test that the thread is not doing any work and it is dead.
-  assert(sharedVar == 0);
-  printf("Main: Done. Successfully killed thread. sharedVar: %d\n", sharedVar);
+  pthread_join(thr, NULL);
   return 0;
 }

test/pthread/test_pthread_kill.out

@@ -1,3 +1,4 @@
-thread startd
-thread killed
-Main: Done. Successfully killed thread. sharedVar: 0
+thread has started, sending SIGTERM
+SIGTERM sent
+signal: 15 onthread=1
+got term signal, shutting down thread