Fix deadlock when calling fflush during application shutdown
One solution to the deadlock described in #15186 is to allow nested
calls to `emscripten_current_thread_process_queued_calls`.
This is needed in this case because the main thread is calling `fflush`
(which requires locking the stdout handle) while another thread is
holding the stdout lock, waiting on the main thread to process the write
action.
There may be reasons we don't want to allow nested calls to
`emscripten_current_thread_process_queued_calls` but technically its
seems possible.
This change removes all the mitigations that existed in
`test_pthread_c11_threads.c` and I could no longer reprodude the
deadlock.
Fixes: #15186
diff --git a/system/lib/pthread/library_pthread.c b/system/lib/pthread/library_pthread.c
index 0a4ee29..55ffec2 100644
--- a/system/lib/pthread/library_pthread.c
+++ b/system/lib/pthread/library_pthread.c
@@ -584,45 +584,34 @@
//emscripten_current_thread_process_queued_calls(), ' + new Error().stack));
// #endif
- static thread_local bool thread_is_processing_queued_calls = false;
-
- // It is possible that when processing a queued call, the control flow leads back to calling this
- // function in a nested fashion! Therefore this scenario must explicitly be detected, and
- // processing the queue must be avoided if we are nesting, or otherwise the same queued calls
- // would be processed again and again.
- if (thread_is_processing_queued_calls)
- return;
- // This must be before pthread_mutex_lock(), since pthread_mutex_lock() can call back to this
- // function.
- thread_is_processing_queued_calls = true;
-
pthread_mutex_lock(&call_queue_lock);
CallQueue* q = GetQueue(pthread_self());
if (!q) {
pthread_mutex_unlock(&call_queue_lock);
- thread_is_processing_queued_calls = false;
return;
}
- int head = q->call_queue_head;
- int tail = q->call_queue_tail;
- while (head != tail) {
+ while (1) {
+ int head = q->call_queue_head;
+ int tail = q->call_queue_tail;
+ if (head == tail) {
+ break;
+ }
+
+ // Remove the item from the queue before running it.
+ em_queued_call* to_run = q->call_queue[head];
+ q->call_queue_head = (head + 1) % CALL_QUEUE_SIZE;
+
// Assume that the call is heavy, so unlock access to the call queue while it is being
// performed.
pthread_mutex_unlock(&call_queue_lock);
- _do_call(q->call_queue[head]);
+ _do_call(to_run);
pthread_mutex_lock(&call_queue_lock);
-
- head = (head + 1) % CALL_QUEUE_SIZE;
- q->call_queue_head = head;
- tail = q->call_queue_tail;
}
pthread_mutex_unlock(&call_queue_lock);
// If the queue was full and we had waiters pending to get to put data to queue, wake them up.
emscripten_futex_wake((void*)&q->call_queue_head, 0x7FFFFFFF);
-
- thread_is_processing_queued_calls = false;
}
// At times when we disallow the main thread to process queued calls, this will
diff --git a/tests/jsrun.py b/tests/jsrun.py
index 684ddd3..72b759f 100644
--- a/tests/jsrun.py
+++ b/tests/jsrun.py
@@ -13,7 +13,7 @@
from tools import shared, utils
WORKING_ENGINES = {} # Holds all configured engines and whether they work: maps path -> True/False
-DEFAULT_TIMEOUT = 5 * 60
+DEFAULT_TIMEOUT = 5
def make_command(filename, engine, args=[]):
diff --git a/tests/pthread/test_pthread_c11_threads.c b/tests/pthread/test_pthread_c11_threads.c
index ac7a463..37a824f 100644
--- a/tests/pthread/test_pthread_c11_threads.c
+++ b/tests/pthread/test_pthread_c11_threads.c
@@ -17,21 +17,6 @@
printf("in do_once\n");
}
-// Because this thread is detached it can still be running
-// when the main thread exits. And becasue of an emscripten
-// bug (https://github.com/emscripten-core/emscripten/issues/15186).
-// this means we can't write to stdout after the main thread
-// exits. This means we can't use `thread_main` below because
-// the destructor to the `tss_t key` writes to stdout.
-int thread_main_detached(void* arg) {
- printf("in thread_main_detached %p\n", (void*)thrd_current());
- mtx_lock(&mutex);
- thread_counter--;
- cnd_signal(&cond);
- mtx_unlock(&mutex);
- return 0;
-}
-
int thread_main(void* arg) {
printf("in thread_main %p\n", (void*)thrd_current());
tss_set(key, (void*)thrd_current());
@@ -102,18 +87,9 @@
// Test thrd_detach
thrd_t t6;
- assert(thrd_create(&t6, thread_main_detached, NULL) == thrd_success);
+ assert(thrd_create(&t6, thread_main, NULL) == thrd_success);
assert(thrd_detach(t6) == thrd_success);
- // Wait for the thread to at least be done printing before exiting
- // the process.
- // We shouldn't need to do this but there is a bug in emscripten
- // where a deadlock can occur between main thread calling fflush()
- // during exitRuntime and the detached thread calling print (and
- // therefore holding the stdout lock).
- // See https://github.com/emscripten-core/emscripten/issues/15186.
- assert(cnd_wait(&cond, &mutex) == thrd_success);
-
printf("done!\n");
return 0;
}
