Skip to content

Commit 5fae227

Browse files
committed
Revert async cancelation behaviour to match musl
This change is partial revert of #19963. We now match musl's behavior which is that async cancellation does *not* work to cancel a thread that is blocked in pthread_mutx_lock. This reverts a few emscripten-specific patches to some very low level parts of musl.
1 parent f5c5bb1 commit 5fae227

File tree

8 files changed

+32
-32
lines changed

8 files changed

+32
-32
lines changed

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ int __timedwait_cp(volatile int *addr, int val,
6161
double msecsToSleep = top ? (top->tv_sec * 1000 + top->tv_nsec / 1000000.0) : INFINITY;
6262

6363
// cp suffix in the function name means "cancellation point", so this wait can be cancelled
64-
// by the users unless current threads cancelability is set to PTHREAD_CANCEL_DISABLE
65-
// which may be either done by the user of __timedwait() function.
64+
// by the user, unless current thread has cancelation disabled (which may be either done
65+
// directly, or indirectly, for example in the __timedwait() function).
6666
pthread_t self = pthread_self();
6767

6868
if (self->canceldisable != PTHREAD_CANCEL_DISABLE) {
@@ -75,15 +75,14 @@ int __timedwait_cp(volatile int *addr, int val,
7575
// __pthread_testcancel(), which won't return at all.
7676
__pthread_testcancel();
7777
// If __pthread_testcancel does return here it means that canceldisable
78-
// must be set to PTHREAD_CANCEL_MASKED. This appears to mean "return
79-
// ECANCELLED to the caller". See pthread_cond_timedwait.c for the only
80-
// use of this that I could find.
78+
// must be set to PTHREAD_CANCEL_MASKED. In this case we emulate the
79+
// behaviour of the futex syscall and return ECANCELLED here.
80+
// See pthread_cond_timedwait.c for the only use of this flag.
8181
return ECANCELED;
8282
}
8383
msecsToSleep = sleepUntilTime - emscripten_get_now();
8484
if (msecsToSleep <= 0) {
85-
r = ETIMEDOUT;
86-
break;
85+
return ETIMEDOUT;
8786
}
8887
// Must wait in slices in case this thread is cancelled in between.
8988
if (msecsToSleep > max_ms_slice_to_sleep)

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

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,7 @@ hidden long __cancel(), __syscall_cp_asm(), __syscall_cp_c();
88
long __cancel()
99
{
1010
pthread_t self = __pthread_self();
11-
#ifdef __EMSCRIPTEN__
12-
// Emscripten doesn't have actual async cancelation so we make a best effort
13-
// by cancelling cooperatively when self->cancelasync is set.
14-
if (self->canceldisable == PTHREAD_CANCEL_ENABLE || self->cancelasync)
15-
#else
1611
if (self->canceldisable == PTHREAD_CANCEL_ENABLE)
17-
#endif
1812
pthread_exit(PTHREAD_CANCELED);
1913
self->canceldisable = PTHREAD_CANCEL_DISABLE;
2014
return -ECANCELED;
@@ -83,12 +77,7 @@ static void cancel_handler(int sig, siginfo_t *si, void *ctx)
8377
void __testcancel()
8478
{
8579
pthread_t self = __pthread_self();
86-
#ifdef __EMSCRIPTEN__
87-
// See comment above about cancelasync under emscripten.
88-
if (self->cancel && (self->cancelasync || !self->canceldisable))
89-
#else
9080
if (self->cancel && !self->canceldisable)
91-
#endif
9281
__cancel();
9382
}
9483

system/lib/pthread/emscripten_futex_wait.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ int emscripten_futex_wait(volatile void *addr, uint32_t val, double max_wait_ms)
132132

133133
#ifdef __EMSCRIPTEN_PTHREADS__
134134
pthread_t self = pthread_self();
135-
bool cancelable = self->cancelasync;
135+
bool cancelable = !self->canceldisable && self->cancelasync;
136136
#else
137137
bool cancelable = false;
138138
#endif

test/codesize/test_codesize_minimal_pthreads.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
22
"a.out.js": 7363,
33
"a.out.js.gz": 3604,
4-
"a.out.nodebug.wasm": 19239,
5-
"a.out.nodebug.wasm.gz": 8916,
6-
"total": 26602,
7-
"total_gz": 12520,
4+
"a.out.nodebug.wasm": 19244,
5+
"a.out.nodebug.wasm.gz": 8919,
6+
"total": 26607,
7+
"total_gz": 12523,
88
"sent": [
99
"a (memory)",
1010
"b (emscripten_get_now)",

test/codesize/test_codesize_minimal_pthreads_memgrowth.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
22
"a.out.js": 7765,
33
"a.out.js.gz": 3810,
4-
"a.out.nodebug.wasm": 19240,
5-
"a.out.nodebug.wasm.gz": 8917,
6-
"total": 27005,
7-
"total_gz": 12727,
4+
"a.out.nodebug.wasm": 19245,
5+
"a.out.nodebug.wasm.gz": 8920,
6+
"total": 27010,
7+
"total_gz": 12730,
88
"sent": [
99
"a (memory)",
1010
"b (emscripten_get_now)",

test/pthread/test_pthread_cancel_async.c

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ void emscripten_outf(const char* msg, ...) {
2626

2727
pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
2828
_Atomic bool started = false;
29+
_Atomic bool timedlock_returned = false;
2930
_Atomic bool done_cleanup = false;
3031

3132
void cleanup_handler(void *arg) {
@@ -47,12 +48,20 @@ void *thread_start(void *arg) {
4748
// Signal the main thread that are started
4849
started = true;
4950

50-
// This mutex is locked by the main thread so this call should never return.
51-
// pthread_mutex_lock is not a cancellation point so deferred cancellation
52-
// won't work here, async cancellation should.
53-
pthread_mutex_lock(&mutex);
51+
// At least under musl, async cancellation also does not work for
52+
// pthread_mutex_lock so this call to pthread_mutex_timedlock should always
53+
// timeout.
54+
struct timespec ts;
55+
clock_gettime(CLOCK_REALTIME, &ts);
56+
ts.tv_sec += 2;
57+
int rc = pthread_mutex_timedlock(&mutex, &ts);
58+
timedlock_returned = true;
59+
assert(rc == ETIMEDOUT);
60+
emscripten_out("pthread_mutex_timedlock timed out");
61+
62+
pthread_testcancel();
5463

55-
assert(false && "pthread_mutex_lock returned!");
64+
assert(false && "pthread_testcancel returned!");
5665
pthread_cleanup_pop(0);
5766
}
5867

@@ -79,6 +88,7 @@ int main() {
7988
emscripten_out("Joining thread..");
8089
s = pthread_join(thr, NULL);
8190
assert(s == 0);
91+
assert(timedlock_returned);
8292
emscripten_out("done");
8393
return 0;
8494
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
Starting thread..
22
Thread started!
33
Canceling thread..
4+
pthread_mutex_timedlock timed out
45
Called clean-up handler with arg 42
56
Joining thread..
67
done

test/test_posixtest.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ def get_tests():
7777
'test_pthread_join_6_3': 'creates too many threads',
7878
'test_pthread_barrier_wait_3_2': 'signals are not supported',
7979
'test_pthread_cond_broadcast_1_2': 'tries to create 10,0000 threads, then depends on fork()',
80+
'test_pthread_setcanceltype_1_1': 'async cancelation does not work withing pthread_mutex_lock',
8081
}
8182

8283
unsupported = {

0 commit comments

Comments
 (0)