Module Name: src Committed By: riastradh Date: Mon Mar 31 14:07:10 UTC 2025
Modified Files: src/lib/libpthread: pthread.c pthread_cancelstub.c pthread_cond.c pthread_int.h src/tests/lib/libpthread: t_cancellation.c Log Message: pthread_cancel(3): Rework. Make pthread_setcancelstate(3) async-signal-safe. (As a side effect, this also makes pthread_setcanceltype(3) async-signal-safe, although that is not required.) PR lib/59134: POSIX-1.2024: pthread_setcancelstate must be async-signal-safe To generate a diff of this commit: cvs rdiff -u -r1.185 -r1.186 src/lib/libpthread/pthread.c cvs rdiff -u -r1.45 -r1.46 src/lib/libpthread/pthread_cancelstub.c cvs rdiff -u -r1.77 -r1.78 src/lib/libpthread/pthread_cond.c cvs rdiff -u -r1.112 -r1.113 src/lib/libpthread/pthread_int.h cvs rdiff -u -r1.1 -r1.2 src/tests/lib/libpthread/t_cancellation.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/lib/libpthread/pthread.c diff -u src/lib/libpthread/pthread.c:1.185 src/lib/libpthread/pthread.c:1.186 --- src/lib/libpthread/pthread.c:1.185 Sat Jun 8 08:01:49 2024 +++ src/lib/libpthread/pthread.c Mon Mar 31 14:07:10 2025 @@ -1,4 +1,4 @@ -/* $NetBSD: pthread.c,v 1.185 2024/06/08 08:01:49 hannken Exp $ */ +/* $NetBSD: pthread.c,v 1.186 2025/03/31 14:07:10 riastradh Exp $ */ /*- * Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008, 2020 @@ -31,7 +31,7 @@ */ #include <sys/cdefs.h> -__RCSID("$NetBSD: pthread.c,v 1.185 2024/06/08 08:01:49 hannken Exp $"); +__RCSID("$NetBSD: pthread.c,v 1.186 2025/03/31 14:07:10 riastradh Exp $"); #define __EXPOSE_STACK 1 @@ -54,6 +54,7 @@ __RCSID("$NetBSD: pthread.c,v 1.185 2024 #include <errno.h> #include <lwp.h> #include <signal.h> +#include <stdatomic.h> #include <stdio.h> #include <stdlib.h> #include <stddef.h> @@ -69,6 +70,15 @@ __RCSID("$NetBSD: pthread.c,v 1.185 2024 #include "pthread_makelwp.h" #include "reentrant.h" +#define atomic_load_relaxed(p) \ + atomic_load_explicit(p, memory_order_relaxed) + +#define atomic_store_relaxed(p, v) \ + atomic_store_explicit(p, v, memory_order_relaxed) + +#define atomic_store_release(p, v) \ + atomic_store_explicit(p, v, memory_order_release) + __BEGIN_DECLS void _malloc_thread_cleanup(void) __weak; __END_DECLS @@ -647,10 +657,7 @@ pthread_exit(void *retval) self = pthread__self(); /* Disable cancellability. */ - pthread_mutex_lock(&self->pt_lock); - self->pt_flags |= PT_FLAG_CS_DISABLED; - self->pt_cancel = 0; - pthread_mutex_unlock(&self->pt_lock); + atomic_store_relaxed(&self->pt_cancel, PT_CANCEL_DISABLED); /* Call any cancellation cleanup handlers */ if (!PTQ_EMPTY(&self->pt_cleanup_stack)) { @@ -868,20 +875,34 @@ pthread_self(void) int pthread_cancel(pthread_t thread) { + unsigned old, new; + bool wake; pthread__error(EINVAL, "Invalid thread", thread->pt_magic == PT_MAGIC); if (pthread__find(thread) != 0) return ESRCH; - pthread_mutex_lock(&thread->pt_lock); - thread->pt_flags |= PT_FLAG_CS_PENDING; - if ((thread->pt_flags & PT_FLAG_CS_DISABLED) == 0) { - thread->pt_cancel = 1; - pthread_mutex_unlock(&thread->pt_lock); + + /* + * membar_release matches membar_acquire in + * pthread_setcancelstate and pthread__testcancel. + */ + membar_release(); + + do { + old = atomic_load_relaxed(&thread->pt_cancel); + new = old | PT_CANCEL_PENDING; + wake = false; + if ((old & PT_CANCEL_DISABLED) == 0) { + new |= PT_CANCEL_CANCELLED; + wake = true; + } + } while (__predict_false(atomic_cas_uint(&thread->pt_cancel, old, new) + != old)); + + if (wake) _lwp_wakeup(thread->pt_lid); - } else - pthread_mutex_unlock(&thread->pt_lock); return 0; } @@ -891,50 +912,76 @@ int pthread_setcancelstate(int state, int *oldstate) { pthread_t self; - int retval; + unsigned flags, old, new; + bool cancelled; if (__predict_false(__uselibcstub)) return __libc_thr_setcancelstate_stub(state, oldstate); self = pthread__self(); - retval = 0; - pthread_mutex_lock(&self->pt_lock); - - if (oldstate != NULL) { - if (self->pt_flags & PT_FLAG_CS_DISABLED) - *oldstate = PTHREAD_CANCEL_DISABLE; - else - *oldstate = PTHREAD_CANCEL_ENABLE; + switch (state) { + case PTHREAD_CANCEL_ENABLE: + flags = 0; + break; + case PTHREAD_CANCEL_DISABLE: + flags = PT_CANCEL_DISABLED; + break; + default: + return EINVAL; } - if (state == PTHREAD_CANCEL_DISABLE) { - self->pt_flags |= PT_FLAG_CS_DISABLED; - if (self->pt_cancel) { - self->pt_flags |= PT_FLAG_CS_PENDING; - self->pt_cancel = 0; - } - } else if (state == PTHREAD_CANCEL_ENABLE) { - self->pt_flags &= ~PT_FLAG_CS_DISABLED; + do { + old = atomic_load_relaxed(&self->pt_cancel); + new = (old & ~PT_CANCEL_DISABLED) | flags; /* + * If we disable while cancelled, switch back to + * pending so future cancellation tests will not fire + * until enabled again. + * * If a cancellation was requested while cancellation * was disabled, note that fact for future * cancellation tests. */ - if (self->pt_flags & PT_FLAG_CS_PENDING) { - self->pt_cancel = 1; + cancelled = false; + if (__predict_false((flags | (old & PT_CANCEL_CANCELLED)) == + (PT_CANCEL_DISABLED|PT_CANCEL_CANCELLED))) { + new &= ~PT_CANCEL_CANCELLED; + new |= PT_CANCEL_PENDING; + } else if (__predict_false((flags | + (old & PT_CANCEL_PENDING)) == + PT_CANCEL_PENDING)) { + new |= PT_CANCEL_CANCELLED; /* This is not a deferred cancellation point. */ - if (self->pt_flags & PT_FLAG_CS_ASYNC) { - pthread_mutex_unlock(&self->pt_lock); - pthread__cancelled(); - } + if (__predict_false(old & PT_CANCEL_ASYNC)) + cancelled = true; } - } else - retval = EINVAL; + } while (__predict_false(atomic_cas_uint(&self->pt_cancel, old, new) + != old)); - pthread_mutex_unlock(&self->pt_lock); + /* + * If we transitioned from PTHREAD_CANCEL_DISABLED to + * PTHREAD_CANCEL_ENABLED, there was a pending cancel, and we + * are configured with asynchronous cancellation, we are now + * cancelled -- make it happen. + */ + if (__predict_false(cancelled)) { + /* + * membar_acquire matches membar_release in + * pthread_cancel. + */ + membar_acquire(); + pthread__cancelled(); + } + + if (oldstate) { + if (old & PT_CANCEL_DISABLED) + *oldstate = PTHREAD_CANCEL_DISABLE; + else + *oldstate = PTHREAD_CANCEL_ENABLE; + } - return retval; + return 0; } @@ -942,34 +989,45 @@ int pthread_setcanceltype(int type, int *oldtype) { pthread_t self; - int retval; + unsigned flags, old, new; + bool cancelled; self = pthread__self(); - retval = 0; - pthread_mutex_lock(&self->pt_lock); + switch (type) { + case PTHREAD_CANCEL_DEFERRED: + flags = 0; + break; + case PTHREAD_CANCEL_ASYNCHRONOUS: + flags = PT_CANCEL_ASYNC; + break; + default: + return EINVAL; + } + + do { + old = atomic_load_relaxed(&self->pt_cancel); + new = (old & ~PT_CANCEL_ASYNC) | flags; + cancelled = false; + if (__predict_false((flags | (old & PT_CANCEL_CANCELLED)) == + (PT_CANCEL_ASYNC|PT_CANCEL_CANCELLED))) + cancelled = true; + } while (__predict_false(atomic_cas_uint(&self->pt_cancel, old, new) + != old)); + + if (__predict_false(cancelled)) { + membar_acquire(); + pthread__cancelled(); + } if (oldtype != NULL) { - if (self->pt_flags & PT_FLAG_CS_ASYNC) + if (old & PT_CANCEL_ASYNC) *oldtype = PTHREAD_CANCEL_ASYNCHRONOUS; else *oldtype = PTHREAD_CANCEL_DEFERRED; } - if (type == PTHREAD_CANCEL_ASYNCHRONOUS) { - self->pt_flags |= PT_FLAG_CS_ASYNC; - if (self->pt_cancel) { - pthread_mutex_unlock(&self->pt_lock); - pthread__cancelled(); - } - } else if (type == PTHREAD_CANCEL_DEFERRED) - self->pt_flags &= ~PT_FLAG_CS_ASYNC; - else - retval = EINVAL; - - pthread_mutex_unlock(&self->pt_lock); - - return retval; + return 0; } @@ -979,8 +1037,8 @@ pthread_testcancel(void) pthread_t self; self = pthread__self(); - if (self->pt_cancel) - pthread__cancelled(); + + pthread__testcancel(self); } @@ -1008,8 +1066,19 @@ void pthread__testcancel(pthread_t self) { - if (self->pt_cancel) + /* + * We use atomic_load_relaxed and then a conditional + * membar_acquire, rather than atomic_load_acquire, in order to + * avoid incurring the cost of an acquire barrier in the common + * case of not having been cancelled. + * + * membar_acquire matches membar_release in pthread_cancel. + */ + if (__predict_false(atomic_load_relaxed(&self->pt_cancel) & + PT_CANCEL_CANCELLED)) { + membar_acquire(); pthread__cancelled(); + } } @@ -1191,7 +1260,9 @@ pthread__park(pthread_t self, pthread_mu } } /* Check for cancellation. */ - if (cancelpt && self->pt_cancel) + if (cancelpt && + (atomic_load_relaxed(&self->pt_cancel) & + PT_CANCEL_CANCELLED)) rv = EINTR; } while (self->pt_sleepobj != NULL && rv == 0); return rv; Index: src/lib/libpthread/pthread_cancelstub.c diff -u src/lib/libpthread/pthread_cancelstub.c:1.45 src/lib/libpthread/pthread_cancelstub.c:1.46 --- src/lib/libpthread/pthread_cancelstub.c:1.45 Fri Jan 19 19:55:03 2024 +++ src/lib/libpthread/pthread_cancelstub.c Mon Mar 31 14:07:10 2025 @@ -1,4 +1,4 @@ -/* $NetBSD: pthread_cancelstub.c,v 1.45 2024/01/19 19:55:03 christos Exp $ */ +/* $NetBSD: pthread_cancelstub.c,v 1.46 2025/03/31 14:07:10 riastradh Exp $ */ /*- * Copyright (c) 2002, 2007 The NetBSD Foundation, Inc. @@ -33,7 +33,7 @@ #undef _FORTIFY_SOURCE #include <sys/cdefs.h> -__RCSID("$NetBSD: pthread_cancelstub.c,v 1.45 2024/01/19 19:55:03 christos Exp $"); +__RCSID("$NetBSD: pthread_cancelstub.c,v 1.46 2025/03/31 14:07:10 riastradh Exp $"); /* Need to use libc-private names for atomic operations. */ #include "../../common/lib/libc/atomic/atomic_op_namespace.h" @@ -65,6 +65,7 @@ __RCSID("$NetBSD: pthread_cancelstub.c,v #include <fcntl.h> #include <mqueue.h> #include <poll.h> +#include <stdatomic.h> #include <stdarg.h> #include <unistd.h> @@ -88,6 +89,9 @@ __RCSID("$NetBSD: pthread_cancelstub.c,v #include "pthread_int.h" #include "reentrant.h" +#define atomic_load_relaxed(p) \ + atomic_load_explicit(p, memory_order_relaxed) + int pthread__cancel_stub_binder; int _sys_accept(int, struct sockaddr *, socklen_t *); @@ -147,8 +151,11 @@ int __sigsuspend14(const sigset_t *); #define TESTCANCEL(id) do { \ if (__predict_true(!__uselibcstub) && \ - __predict_false((id)->pt_cancel)) \ + __predict_false(atomic_load_relaxed(&(id)->pt_cancel) & \ + PT_CANCEL_CANCELLED)) { \ + membar_acquire(); \ pthread__cancelled(); \ + } \ } while (0) Index: src/lib/libpthread/pthread_cond.c diff -u src/lib/libpthread/pthread_cond.c:1.77 src/lib/libpthread/pthread_cond.c:1.78 --- src/lib/libpthread/pthread_cond.c:1.77 Sat Feb 12 14:59:32 2022 +++ src/lib/libpthread/pthread_cond.c Mon Mar 31 14:07:10 2025 @@ -1,4 +1,4 @@ -/* $NetBSD: pthread_cond.c,v 1.77 2022/02/12 14:59:32 riastradh Exp $ */ +/* $NetBSD: pthread_cond.c,v 1.78 2025/03/31 14:07:10 riastradh Exp $ */ /*- * Copyright (c) 2001, 2006, 2007, 2008, 2020 The NetBSD Foundation, Inc. @@ -30,11 +30,12 @@ */ #include <sys/cdefs.h> -__RCSID("$NetBSD: pthread_cond.c,v 1.77 2022/02/12 14:59:32 riastradh Exp $"); +__RCSID("$NetBSD: pthread_cond.c,v 1.78 2025/03/31 14:07:10 riastradh Exp $"); /* Need to use libc-private names for atomic operations. */ #include "../../common/lib/libc/atomic/atomic_op_namespace.h" +#include <stdatomic.h> #include <stdlib.h> #include <errno.h> #include <sys/time.h> @@ -44,6 +45,9 @@ __RCSID("$NetBSD: pthread_cond.c,v 1.77 #include "pthread_int.h" #include "reentrant.h" +#define atomic_load_relaxed(p) \ + atomic_load_explicit(p, memory_order_relaxed) + int _sys___nanosleep50(const struct timespec *, struct timespec *); int _pthread_cond_has_waiters_np(pthread_cond_t *); @@ -139,7 +143,9 @@ pthread_cond_timedwait(pthread_cond_t *c self = pthread__self(); pthread__assert(self->pt_lid != 0); - if (__predict_false(self->pt_cancel)) { + if (__predict_false(atomic_load_relaxed(&self->pt_cancel) & + PT_CANCEL_CANCELLED)) { + membar_acquire(); pthread__cancelled(); } @@ -166,7 +172,9 @@ pthread_cond_timedwait(pthread_cond_t *c /* Drop the interlock and wait. */ error = 0; pthread_mutex_unlock(mutex); - while (waiter.lid && !(cancel = self->pt_cancel)) { + while (waiter.lid && + !(cancel = atomic_load_relaxed(&self->pt_cancel) & + PT_CANCEL_CANCELLED)) { int rv = _lwp_park(clkid, TIMER_ABSTIME, __UNCONST(abstime), 0, NULL, NULL); if (rv == 0) { @@ -209,6 +217,7 @@ pthread_cond_timedwait(pthread_cond_t *c * held if this happens. */ if (cancel) { + membar_acquire(); pthread__cancelled(); } Index: src/lib/libpthread/pthread_int.h diff -u src/lib/libpthread/pthread_int.h:1.112 src/lib/libpthread/pthread_int.h:1.113 --- src/lib/libpthread/pthread_int.h:1.112 Sat Nov 30 01:04:04 2024 +++ src/lib/libpthread/pthread_int.h Mon Mar 31 14:07:10 2025 @@ -1,4 +1,4 @@ -/* $NetBSD: pthread_int.h,v 1.112 2024/11/30 01:04:04 christos Exp $ */ +/* $NetBSD: pthread_int.h,v 1.113 2025/03/31 14:07:10 riastradh Exp $ */ /*- * Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008, 2020 @@ -96,7 +96,7 @@ struct __pthread_st { unsigned int pt_magic; /* Magic number */ int pt_state; /* running, blocked, etc. */ int pt_flags; /* see PT_FLAG_* below */ - int pt_cancel; /* Deferred cancellation */ + volatile unsigned int pt_cancel; /* Cancellation */ int pt_errno; /* Thread-specific errno. */ stack_t pt_stack; /* Our stack */ bool pt_stack_allocated; @@ -152,13 +152,17 @@ struct __pthread_st { /* Flag values */ #define PT_FLAG_DETACHED 0x0001 -#define PT_FLAG_CS_DISABLED 0x0004 /* Cancellation disabled */ -#define PT_FLAG_CS_ASYNC 0x0008 /* Cancellation is async */ -#define PT_FLAG_CS_PENDING 0x0010 #define PT_FLAG_SCOPE_SYSTEM 0x0040 #define PT_FLAG_EXPLICIT_SCHED 0x0080 #define PT_FLAG_SUSPENDED 0x0100 /* In the suspended queue */ +/* pt_cancel word */ + +#define PT_CANCEL_DISABLED __BIT(0) +#define PT_CANCEL_ASYNC __BIT(1) +#define PT_CANCEL_PENDING __BIT(2) +#define PT_CANCEL_CANCELLED __BIT(3) + #define PT_MAGIC 0x11110001 #define PT_DEAD 0xDEAD0001 Index: src/tests/lib/libpthread/t_cancellation.c diff -u src/tests/lib/libpthread/t_cancellation.c:1.1 src/tests/lib/libpthread/t_cancellation.c:1.2 --- src/tests/lib/libpthread/t_cancellation.c:1.1 Mon Mar 31 13:57:06 2025 +++ src/tests/lib/libpthread/t_cancellation.c Mon Mar 31 14:07:10 2025 @@ -1,4 +1,4 @@ -/* $NetBSD: t_cancellation.c,v 1.1 2025/03/31 13:57:06 riastradh Exp $ */ +/* $NetBSD: t_cancellation.c,v 1.2 2025/03/31 14:07:10 riastradh Exp $ */ /* * Copyright (c) 2025 The NetBSD Foundation, Inc. @@ -27,7 +27,7 @@ */ #include <sys/cdefs.h> -__RCSID("$NetBSD: t_cancellation.c,v 1.1 2025/03/31 13:57:06 riastradh Exp $"); +__RCSID("$NetBSD: t_cancellation.c,v 1.2 2025/03/31 14:07:10 riastradh Exp $"); #include <sys/mman.h> #include <sys/msg.h> @@ -1497,8 +1497,6 @@ ATF_TC_BODY(sigsafecancelstate, tc) memory_order_relaxed); } - atf_tc_expect_signal(SIGALRM, "PR lib/59134: POSIX-1.2024:" - " pthread_setcancelstate must be async-signal-safe"); alarm(1); RZ(pthread_join(t, &result)); ATF_CHECK_MSG(result == NULL, "result=%p", result); @@ -1619,4 +1617,3 @@ ATF_TP_ADD_TCS(tp) return atf_no_error(); } -