Skip to content

Commit a8d3d3f

Browse files
committed
Add pthread mutex deadlock detection for debug builds
This would really have helped in at least detecting the deadlock that was found in #24570 / #24565
1 parent cc1de1a commit a8d3d3f

File tree

6 files changed

+47
-4
lines changed

6 files changed

+47
-4
lines changed

system/lib/libc/musl/src/thread/pthread_mutex_lock.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@
22

33
int __pthread_mutex_lock(pthread_mutex_t *m)
44
{
5+
#if !defined(__EMSCRIPTEN__) || defined(NDEBUG)
6+
/* XXX EMSCRIPTEN always take the slow path in debug builds so we can trap rather than deadlock */
57
if ((m->_m_type&15) == PTHREAD_MUTEX_NORMAL
68
&& !a_cas(&m->_m_lock, 0, EBUSY))
79
return 0;
10+
#endif
811

912
return __pthread_mutex_timedlock(m, 0);
1013
}

system/lib/libc/musl/src/thread/pthread_mutex_timedlock.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
#include "pthread_impl.h"
22

3+
#ifdef __EMSCRIPTEN__
4+
#include <assert.h>
5+
#endif
6+
37
#ifndef __EMSCRIPTEN__
48
#define IS32BIT(x) !((x)+0x80000000ULL>>32)
59
#define CLAMP(x) (int)(IS32BIT(x) ? (x) : 0x7fffffffU+((0ULL+(x))>>63))
@@ -57,9 +61,12 @@ static int pthread_mutex_timedlock_pi(pthread_mutex_t *restrict m, const struct
5761

5862
int __pthread_mutex_timedlock(pthread_mutex_t *restrict m, const struct timespec *restrict at)
5963
{
64+
#if !defined(__EMSCRIPTEN__) || defined(NDEBUG)
65+
/* XXX EMSCRIPTEN always take the slow path in debug builds so we can trap rather than deadlock */
6066
if ((m->_m_type&15) == PTHREAD_MUTEX_NORMAL
6167
&& !a_cas(&m->_m_lock, 0, EBUSY))
6268
return 0;
69+
#endif
6370

6471
int type = m->_m_type;
6572
int r, t, priv = (type & 128) ^ 128;
@@ -79,6 +86,9 @@ int __pthread_mutex_timedlock(pthread_mutex_t *restrict m, const struct timespec
7986
int own = r & 0x3fffffff;
8087
if (!own && (!r || (type&4)))
8188
continue;
89+
#if defined(__EMSCRIPTEN__) && !defined(NDEBUG)
90+
assert(own != __pthread_self()->tid && "pthread mutex deadlock detected");
91+
#endif
8292
if ((type&3) == PTHREAD_MUTEX_ERRORCHECK
8393
&& own == __pthread_self()->tid)
8494
return EDEADLK;

system/lib/libc/musl/src/thread/pthread_mutex_trylock.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,11 @@ int __pthread_mutex_trylock_owner(pthread_mutex_t *m)
7171

7272
int __pthread_mutex_trylock(pthread_mutex_t *m)
7373
{
74+
#if !defined(__EMSCRIPTEN__) || defined(NDEBUG)
75+
/* XXX EMSCRIPTEN always take the slow path in debug builds so we can trap rather than deadlock */
7476
if ((m->_m_type&15) == PTHREAD_MUTEX_NORMAL)
7577
return a_cas(&m->_m_lock, 0, EBUSY) & EBUSY;
78+
#endif
7679
return __pthread_mutex_trylock_owner(m);
7780
}
7881

system/lib/pthread/library_pthread.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,13 +138,9 @@ weak_alias(dummy_tsd, __pthread_tsd_main);
138138
// See system/lib/README.md for static constructor ordering.
139139
__attribute__((constructor(48)))
140140
void _emscripten_init_main_thread(void) {
141-
_emscripten_init_main_thread_js(&__main_pthread);
142-
143141
// The pthread struct has a field that points to itself - this is used as
144142
// a magic ID to detect whether the pthread_t structure is 'alive'.
145143
__main_pthread.self = &__main_pthread;
146-
__main_pthread.stack = (void*)emscripten_stack_get_base();
147-
__main_pthread.stack_size = emscripten_stack_get_base() - emscripten_stack_get_end();
148144
__main_pthread.detach_state = DT_JOINABLE;
149145
// pthread struct robust_list head should point to itself.
150146
__main_pthread.robust_list.head = &__main_pthread.robust_list.head;
@@ -157,6 +153,11 @@ void _emscripten_init_main_thread(void) {
157153
__main_pthread.next = __main_pthread.prev = &__main_pthread;
158154
__main_pthread.tsd = (void **)__pthread_tsd_main;
159155

156+
_emscripten_init_main_thread_js(&__main_pthread);
157+
158+
__main_pthread.stack = (void*)emscripten_stack_get_base();
159+
__main_pthread.stack_size = emscripten_stack_get_base() - emscripten_stack_get_end();
160+
160161
_emscripten_thread_mailbox_init(&__main_pthread);
161162
_emscripten_thread_mailbox_await(&__main_pthread);
162163
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#include <assert.h>
2+
#include <pthread.h>
3+
#include <stdbool.h>
4+
#include <stdio.h>
5+
6+
pthread_mutex_t m = PTHREAD_MUTEX_INITIALIZER;
7+
8+
int main() {
9+
printf("in main\n");
10+
int rtn = pthread_mutex_lock(&m);
11+
assert(rtn == 0);
12+
13+
// Attempt to lock a second time. In debug builds this should
14+
// hit an assertion. In release builds this will deadlock and
15+
// never return.
16+
pthread_mutex_lock(&m);
17+
printf("should never get here\n");
18+
assert(false);
19+
return 0;
20+
}

test/test_other.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12946,6 +12946,12 @@ def test_pthread_reuse(self):
1294612946
def test_pthread_hello(self, args):
1294712947
self.do_other_test('test_pthread_hello.c', args)
1294812948

12949+
@crossplatform
12950+
@node_pthreads
12951+
def test_pthread_mutex_deadlock(self):
12952+
self.do_runf('other/test_pthread_mutex_deadlock.c', 'pthread mutex deadlock detected',
12953+
cflags=['-g'], assert_returncode=NON_ZERO)
12954+
1294912955
@node_pthreads
1295012956
def test_pthread_relocatable(self):
1295112957
self.do_run_in_out_file_test('hello_world.c', cflags=['-sRELOCATABLE'])

0 commit comments

Comments
 (0)