Skip to content

Commit 2179619

Browse files
committed
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
1 parent bca56b3 commit 2179619

File tree

3 files changed

+14
-49
lines changed

3 files changed

+14
-49
lines changed

system/lib/pthread/library_pthread.c

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -584,45 +584,34 @@ void emscripten_current_thread_process_queued_calls() {
584584
//emscripten_current_thread_process_queued_calls(), ' + new Error().stack));
585585
// #endif
586586

587-
static thread_local bool thread_is_processing_queued_calls = false;
588-
589-
// It is possible that when processing a queued call, the control flow leads back to calling this
590-
// function in a nested fashion! Therefore this scenario must explicitly be detected, and
591-
// processing the queue must be avoided if we are nesting, or otherwise the same queued calls
592-
// would be processed again and again.
593-
if (thread_is_processing_queued_calls)
594-
return;
595-
// This must be before pthread_mutex_lock(), since pthread_mutex_lock() can call back to this
596-
// function.
597-
thread_is_processing_queued_calls = true;
598-
599587
pthread_mutex_lock(&call_queue_lock);
600588
CallQueue* q = GetQueue(pthread_self());
601589
if (!q) {
602590
pthread_mutex_unlock(&call_queue_lock);
603-
thread_is_processing_queued_calls = false;
604591
return;
605592
}
606593

607-
int head = q->call_queue_head;
608-
int tail = q->call_queue_tail;
609-
while (head != tail) {
594+
while (1) {
595+
int head = q->call_queue_head;
596+
int tail = q->call_queue_tail;
597+
if (head == tail) {
598+
break;
599+
}
600+
601+
// Remove the item from the queue before running it.
602+
em_queued_call* to_run = q->call_queue[head];
603+
q->call_queue_head = (head + 1) % CALL_QUEUE_SIZE;
604+
610605
// Assume that the call is heavy, so unlock access to the call queue while it is being
611606
// performed.
612607
pthread_mutex_unlock(&call_queue_lock);
613-
_do_call(q->call_queue[head]);
608+
_do_call(to_run);
614609
pthread_mutex_lock(&call_queue_lock);
615-
616-
head = (head + 1) % CALL_QUEUE_SIZE;
617-
q->call_queue_head = head;
618-
tail = q->call_queue_tail;
619610
}
620611
pthread_mutex_unlock(&call_queue_lock);
621612

622613
// If the queue was full and we had waiters pending to get to put data to queue, wake them up.
623614
emscripten_futex_wake((void*)&q->call_queue_head, 0x7FFFFFFF);
624-
625-
thread_is_processing_queued_calls = false;
626615
}
627616

628617
// At times when we disallow the main thread to process queued calls, this will

tests/jsrun.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from tools import shared, utils
1414

1515
WORKING_ENGINES = {} # Holds all configured engines and whether they work: maps path -> True/False
16-
DEFAULT_TIMEOUT = 5 * 60
16+
DEFAULT_TIMEOUT = 5
1717

1818

1919
def make_command(filename, engine, args=[]):

tests/pthread/test_pthread_c11_threads.c

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,6 @@ void do_once(void) {
1717
printf("in do_once\n");
1818
}
1919

20-
// Because this thread is detached it can still be running
21-
// when the main thread exits. And becasue of an emscripten
22-
// bug (https://github.com/emscripten-core/emscripten/issues/15186).
23-
// this means we can't write to stdout after the main thread
24-
// exits. This means we can't use `thread_main` below because
25-
// the destructor to the `tss_t key` writes to stdout.
26-
int thread_main_detached(void* arg) {
27-
printf("in thread_main_detached %p\n", (void*)thrd_current());
28-
mtx_lock(&mutex);
29-
thread_counter--;
30-
cnd_signal(&cond);
31-
mtx_unlock(&mutex);
32-
return 0;
33-
}
34-
3520
int thread_main(void* arg) {
3621
printf("in thread_main %p\n", (void*)thrd_current());
3722
tss_set(key, (void*)thrd_current());
@@ -102,18 +87,9 @@ int main(int argc, char* argv[]) {
10287

10388
// Test thrd_detach
10489
thrd_t t6;
105-
assert(thrd_create(&t6, thread_main_detached, NULL) == thrd_success);
90+
assert(thrd_create(&t6, thread_main, NULL) == thrd_success);
10691
assert(thrd_detach(t6) == thrd_success);
10792

108-
// Wait for the thread to at least be done printing before exiting
109-
// the process.
110-
// We shouldn't need to do this but there is a bug in emscripten
111-
// where a deadlock can occur between main thread calling fflush()
112-
// during exitRuntime and the detached thread calling print (and
113-
// therefore holding the stdout lock).
114-
// See https://github.com/emscripten-core/emscripten/issues/15186.
115-
assert(cnd_wait(&cond, &mutex) == thrd_success);
116-
11793
printf("done!\n");
11894
return 0;
11995
}

0 commit comments

Comments
 (0)