Fix flake in test_pthread_proxying_refcount (#25097)

It apparently takes more than one turn of the worker thread's event loop
to ensure that the notifications on the zombie task queues have been
cleared so the zombies can be culled. Update the test to wait two turns
of the event loop instead of one.

Also add new synchronization forcing the main thread to wait until the
worker thread has entered Wasm before proxying work to it. This prevents
the proxied work notifications from somehow being cleared before the
worker thread destroys the proxy queues, which would prevent the task
queues from ever being placed on the zombie list in the first place.

Finally, generally improve comments and make the test assertions more
specific.

Fixes #19795.

Author: tlively

test/pthread/test_pthread_proxying_refcount.c

@@ -5,9 +5,14 @@
 #include <emscripten/proxying.h>
 #include <pthread.h>
 #include <sched.h>
+#include <stdatomic.h>
 #include <stdbool.h>
 #include <unistd.h>
 
+// Test that task_queues destroyed with pending notifications are added to the
+// zombie list and are culled when a new task queue is allocated after their
+// notifications have been cleared.
+
 #if __has_feature(leak_sanitizer) || __has_feature(address_sanitizer)
 #define SANITIZER
 #endif
@@ -29,22 +34,28 @@ void __attribute__((noinline)) free(void* ptr) {
 
 #endif // SANITIZER
 
-_Atomic int should_execute = 0;
+_Atomic bool worker_started = false;
+_Atomic bool should_execute = false;
 _Atomic int executed[2] = {};
-_Atomic int processed = 0;
 
-EMSCRIPTEN_KEEPALIVE
-void register_processed(void) {
-  processed++;
-}
+#define EVENT_LOOP_TURNS 2
 
-void set_flag(void* arg) { *(_Atomic int*)arg = 1; }
+// Delay culling until the event loop has turned enough for the notifications on
+// the two zombie task queues to have been received and cleared, allowing them
+// to be freed.
+void increment_flag(void* arg) {
+  _Atomic int* flag = arg;
+  if (atomic_fetch_add(flag, 1) + 1 < EVENT_LOOP_TURNS) {
+    emscripten_async_call(increment_flag, arg, 0);
+  }
+}
 
-// Delay setting the flag until the next turn of the event loop so it can be set
-// after the proxying queue is destroyed.
-void task(void* arg) { emscripten_async_call(set_flag, arg, 0); }
+void task(void* arg) { emscripten_async_call(increment_flag, arg, 0); }
 
 void* execute_and_free_queue(void* arg) {
+  // Signal the main thread to proxy work to us.
+  worker_started = true;
+
   // Wait until we are signaled to execute the queue.
   while (!should_execute) {
     sched_yield();
@@ -56,10 +67,8 @@ void* execute_and_free_queue(void* arg) {
     em_proxying_queue_destroy(queues[i]);
   }
 
-  // Exit with a live runtime so the queued work notification is received and we
-  // try to execute the queue again, even though we already executed all its
-  // work and we are now just waiting for the notifications to be received so we
-  // can free it.
+  // Exit with a live runtime so the queued work notification can be received
+  // and cleared, allowing the zombie task queues to be culled.
   emscripten_exit_with_live_runtime();
 }
 
@@ -72,17 +81,25 @@ int main() {
     assert(queues[i]);
   }
 
-  // Create the worker and send it tasks.
+  // Create the worker and wait for it to enter Wasm.
   pthread_t worker;
   pthread_create(&worker, NULL, execute_and_free_queue, NULL);
+  while (!worker_started) {
+    sched_yield();
+  }
+
+  // Now that the worker has started, proxy work to it. This will allocate task
+  // queues and send the worker notifications. The worker will execute the tasks
+  // and destroy the proxy queues before returning to the event loop to receive
+  // the notifications, so the task queues will end up on the zombie list.
   for (int i = 0; i < 2; i++) {
     emscripten_proxy_async(queues[i], worker, task, &executed[i]);
   }
-  should_execute = 1;
+  should_execute = true;
 
-  // Wait for the tasks to be executed. The queues will have been destroyed
-  // after this.
-  while (!executed[0] || !executed[1]) {
+  // Wait for the queues to be destroyed and for the worker event loop to turn
+  // enough to clear the notifications
+  while (executed[0] < EVENT_LOOP_TURNS || executed[1] < EVENT_LOOP_TURNS) {
     sched_yield();
   }
 
@@ -99,6 +116,9 @@ int main() {
   // Now they should be free.
   int frees_after_cull = frees;
   assert(frees_after_cull > frees_before_cull);
+  // For each of the two culled task queues, the queue itself and its task
+  // vector should have been freed.
+  assert(frees_after_cull - frees_before_cull == 4);
 #endif // SANITIZER
 
   // If we try again, there should be nothing left to cull.