Skip to content

Commit 42fb00c

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 d8a6db2 commit 42fb00c

File tree

8 files changed

+57
-6
lines changed

8 files changed

+57
-6
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: 12 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;
@@ -82,6 +89,11 @@ int __pthread_mutex_timedlock(pthread_mutex_t *restrict m, const struct timespec
8289
if ((type&3) == PTHREAD_MUTEX_ERRORCHECK
8390
&& own == __pthread_self()->tid)
8491
return EDEADLK;
92+
#if defined(__EMSCRIPTEN__) && !defined(NDEBUG)
93+
// Extra check for deadlock in debug builds, but only if we would block
94+
// forever (at == NULL).
95+
assert(at || own != __pthread_self()->tid && "pthread mutex deadlock detected");
96+
#endif
8597

8698
a_inc(&m->_m_waiters);
8799
t = r | 0x80000000;

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ int __pthread_mutex_trylock_owner(pthread_mutex_t *m)
5353
}
5454
#endif
5555

56+
#if defined(__EMSCRIPTEN__) || !defined(NDEBUG)
57+
// We can get here for normal mutexes too, but only in debug builds
58+
// (where we track ownership purely for debug purposed).
59+
if ((type & 15) == PTHREAD_MUTEX_NORMAL) return 0;
60+
#endif
61+
5662
next = self->robust_list.head;
5763
m->_m_next = next;
5864
m->_m_prev = &self->robust_list.head;
@@ -71,8 +77,11 @@ int __pthread_mutex_trylock_owner(pthread_mutex_t *m)
7177

7278
int __pthread_mutex_trylock(pthread_mutex_t *m)
7379
{
80+
#if !defined(__EMSCRIPTEN__) || defined(NDEBUG)
81+
/* XXX EMSCRIPTEN always take the slow path in debug builds so we can trap rather than deadlock */
7482
if ((m->_m_type&15) == PTHREAD_MUTEX_NORMAL)
7583
return a_cas(&m->_m_lock, 0, EBUSY) & EBUSY;
84+
#endif
7685
return __pthread_mutex_trylock_owner(m);
7786
}
7887

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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
19526
1+
19540
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
19527
1+
19541
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
@@ -12947,6 +12947,12 @@ def test_pthread_reuse(self):
1294712947
def test_pthread_hello(self, args):
1294812948
self.do_other_test('test_pthread_hello.c', args)
1294912949

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

0 commit comments

Comments
 (0)