Skip to content
Open
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -602,4 +602,5 @@ a license to everyone to use it as detailed in LICENSE.)
* Christian Lloyd <clloyd@teladochealth.com> (copyright owned by Teladoc Health, Inc.)
* Sean Morris <sean@seanmorr.is>
* Mitchell Wills <mwills@google.com> (copyright owned by Google, Inc.)
* Eugene Hopkinson <slowriot@armchair.software>
* Han Jiang <jhcarl0814@gmail.com>
3 changes: 0 additions & 3 deletions system/lib/libc/musl/src/thread/pthread_mutex_lock.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@

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

return __pthread_mutex_timedlock(m, 0);
}
Expand Down
12 changes: 0 additions & 12 deletions system/lib/libc/musl/src/thread/pthread_mutex_timedlock.c
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
#include "pthread_impl.h"

#ifdef __EMSCRIPTEN__
#include <assert.h>
#endif

#ifndef __EMSCRIPTEN__
#define IS32BIT(x) !((x)+0x80000000ULL>>32)
#define CLAMP(x) (int)(IS32BIT(x) ? (x) : 0x7fffffffU+((0ULL+(x))>>63))
Expand Down Expand Up @@ -61,12 +57,9 @@ static int pthread_mutex_timedlock_pi(pthread_mutex_t *restrict m, const struct

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

int type = m->_m_type;
int r, t, priv = (type & 128) ^ 128;
Expand All @@ -89,11 +82,6 @@ int __pthread_mutex_timedlock(pthread_mutex_t *restrict m, const struct timespec
if ((type&3) == PTHREAD_MUTEX_ERRORCHECK
&& own == __pthread_self()->tid)
return EDEADLK;
#if defined(__EMSCRIPTEN__) && !defined(NDEBUG)
// Extra check for deadlock in debug builds, but only if we would block
// forever (at == NULL).
assert(at || own != __pthread_self()->tid && "pthread mutex deadlock detected");
#endif

a_inc(&m->_m_waiters);
t = r | 0x80000000;
Expand Down
9 changes: 0 additions & 9 deletions system/lib/libc/musl/src/thread/pthread_mutex_trylock.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,6 @@ int __pthread_mutex_trylock_owner(pthread_mutex_t *m)
}
#endif

#if defined(__EMSCRIPTEN__) || !defined(NDEBUG)
// We can get here for normal mutexes too, but only in debug builds
// (where we track ownership purely for debug purposes).
if ((type & 15) == PTHREAD_MUTEX_NORMAL) return 0;
#endif

next = self->robust_list.head;
m->_m_next = next;
m->_m_prev = &self->robust_list.head;
Expand All @@ -77,11 +71,8 @@ int __pthread_mutex_trylock_owner(pthread_mutex_t *m)

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

Expand Down
8 changes: 4 additions & 4 deletions test/codesize/test_codesize_minimal_pthreads.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"a.out.js": 7367,
"a.out.js.gz": 3603,
"a.out.nodebug.wasm": 19003,
"a.out.nodebug.wasm.gz": 8786,
"total": 26370,
"total_gz": 12389,
"a.out.nodebug.wasm": 18991,
"a.out.nodebug.wasm.gz": 8771,
"total": 26358,
"total_gz": 12374,
"sent": [
"a (memory)",
"b (exit)",
Expand Down
8 changes: 4 additions & 4 deletions test/codesize/test_codesize_minimal_pthreads_memgrowth.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"a.out.js": 7769,
"a.out.js.gz": 3809,
"a.out.nodebug.wasm": 19004,
"a.out.nodebug.wasm.gz": 8787,
"total": 26773,
"total_gz": 12596,
"a.out.nodebug.wasm": 18992,
"a.out.nodebug.wasm.gz": 8772,
"total": 26761,
"total_gz": 12581,
"sent": [
"a (memory)",
"b (exit)",
Expand Down
4 changes: 4 additions & 0 deletions test/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -5243,6 +5243,10 @@ def test_wasm_worker_proxied_function(self):
# Test that code does not crash in ASSERTIONS-disabled builds
self.btest('wasm_worker/proxied_function.c', expected='0', cflags=['--js-library', test_file('wasm_worker/proxied_function.js'), '-sWASM_WORKERS', '-sASSERTIONS=0'])

def test_wasm_worker_pthread_mutex_debug_allocator_regression(self):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a link to https://github.com/emscripten-core/emscripten/issues/26619 here?

(And also in the test source code).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in latest (in case this branch is still potentially useful)

self.btest_exit('wasm_worker/pthread_mutex_debug_allocator_regression.c',
cflags=['-pthread', '-sWASM_WORKERS'])
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this into test_other.py? I had to add -sEXIT_RUNTIME and emscripten_terminate_all_wasm_workers to make it work.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in latest (in case this branch is still potentially useful)


@no_firefox('no 4GB support yet')
@no_2gb('uses MAXIMUM_MEMORY')
@no_4gb('uses MAXIMUM_MEMORY')
Expand Down
4 changes: 4 additions & 0 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -11723,6 +11723,10 @@ def test_pthread_reuse(self):
def test_pthread_hello(self, args):
self.do_other_test('test_pthread_hello.c', args)

# Preserved for future reinstatement of the debug deadlock check from #24607.
# This rollback intentionally removes that behavior and returns debug builds to
# the pre-4.0.11 fast-path behavior for PTHREAD_MUTEX_NORMAL.
@disabled('disabled by revert-pthread-mutex-debug-deadlock-detection rollback')
@crossplatform
@requires_pthreads
def test_pthread_mutex_deadlock(self):
Expand Down
24 changes: 24 additions & 0 deletions test/wasm_worker/pthread_mutex_debug_allocator_regression.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#include <stdlib.h>

#include <emscripten.h>
#include <emscripten/wasm_worker.h>

static void worker_loop(void) {
for (;;) {
free(malloc(1));
}
}

static void main_loop(void) {
static unsigned ticks;
malloc(1);
if (++ticks == 120) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to add emscripten_terminate_all_wasm_workers there to make the test actually return.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use your C repro in latest (in case this branch is still potentially useful)

emscripten_force_exit(0);
}
}

int main(void) {
emscripten_wasm_worker_post_function_v(emscripten_malloc_wasm_worker(1024 * 1024), worker_loop);
emscripten_set_main_loop(main_loop, 0, false);
return 0;
}
Loading