Tue Apr 14 23:35:07 2020 UTC ()
Drop most of the logic associated with pthread__started.

The pthread_cond logic is a questionable optimisation at best and the
post-fork logic is plainly broken.


(joerg)
diff -r1.167 -r1.168 src/lib/libpthread/pthread.c
diff -r1.67 -r1.68 src/lib/libpthread/pthread_cond.c

cvs diff -r1.167 -r1.168 src/lib/libpthread/pthread.c (expand / switch to unified diff)

--- src/lib/libpthread/pthread.c 2020/02/16 17:45:11 1.167
+++ src/lib/libpthread/pthread.c 2020/04/14 23:35:07 1.168
@@ -1,14 +1,14 @@ @@ -1,14 +1,14 @@
1/* $NetBSD: pthread.c,v 1.167 2020/02/16 17:45:11 kamil Exp $ */ 1/* $NetBSD: pthread.c,v 1.168 2020/04/14 23:35:07 joerg Exp $ */
2 2
3/*- 3/*-
4 * Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008, 2020 4 * Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008, 2020
5 * The NetBSD Foundation, Inc. 5 * The NetBSD Foundation, Inc.
6 * All rights reserved. 6 * All rights reserved.
7 * 7 *
8 * This code is derived from software contributed to The NetBSD Foundation 8 * This code is derived from software contributed to The NetBSD Foundation
9 * by Nathan J. Williams and Andrew Doran. 9 * by Nathan J. Williams and Andrew Doran.
10 * 10 *
11 * Redistribution and use in source and binary forms, with or without 11 * Redistribution and use in source and binary forms, with or without
12 * modification, are permitted provided that the following conditions 12 * modification, are permitted provided that the following conditions
13 * are met: 13 * are met:
14 * 1. Redistributions of source code must retain the above copyright 14 * 1. Redistributions of source code must retain the above copyright
@@ -21,27 +21,27 @@ @@ -21,27 +21,27 @@
21 * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED 21 * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
22 * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR 22 * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
23 * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS 23 * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS
24 * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR 24 * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
25 * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF 25 * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
26 * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS 26 * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
27 * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN 27 * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
28 * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) 28 * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
29 * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE 29 * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
30 * POSSIBILITY OF SUCH DAMAGE. 30 * POSSIBILITY OF SUCH DAMAGE.
31 */ 31 */
32 32
33#include <sys/cdefs.h> 33#include <sys/cdefs.h>
34__RCSID("$NetBSD: pthread.c,v 1.167 2020/02/16 17:45:11 kamil Exp $"); 34__RCSID("$NetBSD: pthread.c,v 1.168 2020/04/14 23:35:07 joerg Exp $");
35 35
36#define __EXPOSE_STACK 1 36#define __EXPOSE_STACK 1
37 37
38#include <sys/param.h> 38#include <sys/param.h>
39#include <sys/exec_elf.h> 39#include <sys/exec_elf.h>
40#include <sys/mman.h> 40#include <sys/mman.h>
41#include <sys/lwp.h> 41#include <sys/lwp.h>
42#include <sys/lwpctl.h> 42#include <sys/lwpctl.h>
43#include <sys/resource.h> 43#include <sys/resource.h>
44#include <sys/sysctl.h> 44#include <sys/sysctl.h>
45#include <sys/tls.h> 45#include <sys/tls.h>
46#include <uvm/uvm_param.h> 46#include <uvm/uvm_param.h>
47 47
@@ -74,28 +74,26 @@ static signed int pthread__cmp(void *, c @@ -74,28 +74,26 @@ static signed int pthread__cmp(void *, c
74static const rb_tree_ops_t pthread__alltree_ops = { 74static const rb_tree_ops_t pthread__alltree_ops = {
75 .rbto_compare_nodes = pthread__cmp, 75 .rbto_compare_nodes = pthread__cmp,
76 .rbto_compare_key = pthread__cmp, 76 .rbto_compare_key = pthread__cmp,
77 .rbto_node_offset = offsetof(struct __pthread_st, pt_alltree), 77 .rbto_node_offset = offsetof(struct __pthread_st, pt_alltree),
78 .rbto_context = NULL 78 .rbto_context = NULL
79}; 79};
80 80
81static void pthread__create_tramp(void *); 81static void pthread__create_tramp(void *);
82static void pthread__initthread(pthread_t); 82static void pthread__initthread(pthread_t);
83static void pthread__scrubthread(pthread_t, char *, int); 83static void pthread__scrubthread(pthread_t, char *, int);
84static void pthread__initmain(pthread_t *); 84static void pthread__initmain(pthread_t *);
85static void pthread__fork_callback(void); 85static void pthread__fork_callback(void);
86static void pthread__reap(pthread_t); 86static void pthread__reap(pthread_t);
87static void pthread__child_callback(void); 
88static void pthread__start(void); 
89 87
90void pthread__init(void); 88void pthread__init(void);
91 89
92int pthread__started; 90int pthread__started;
93int __uselibcstub = 1; 91int __uselibcstub = 1;
94pthread_mutex_t pthread__deadqueue_lock = PTHREAD_MUTEX_INITIALIZER; 92pthread_mutex_t pthread__deadqueue_lock = PTHREAD_MUTEX_INITIALIZER;
95pthread_queue_t pthread__deadqueue; 93pthread_queue_t pthread__deadqueue;
96pthread_queue_t pthread__allqueue; 94pthread_queue_t pthread__allqueue;
97 95
98static pthread_attr_t pthread_default_attr; 96static pthread_attr_t pthread_default_attr;
99static lwpctl_t pthread__dummy_lwpctl = { .lc_curcpu = LWPCTL_CPU_NONE }; 97static lwpctl_t pthread__dummy_lwpctl = { .lc_curcpu = LWPCTL_CPU_NONE };
100 98
101enum { 99enum {
@@ -264,56 +262,26 @@ pthread__init(void) @@ -264,56 +262,26 @@ pthread__init(void)
264 262
265static void 263static void
266pthread__fork_callback(void) 264pthread__fork_callback(void)
267{ 265{
268 struct __pthread_st *self = pthread__self(); 266 struct __pthread_st *self = pthread__self();
269 267
270 /* lwpctl state is not copied across fork. */ 268 /* lwpctl state is not copied across fork. */
271 if (_lwp_ctl(LWPCTL_FEATURE_CURCPU, &self->pt_lwpctl)) { 269 if (_lwp_ctl(LWPCTL_FEATURE_CURCPU, &self->pt_lwpctl)) {
272 err(EXIT_FAILURE, "_lwp_ctl"); 270 err(EXIT_FAILURE, "_lwp_ctl");
273 } 271 }
274 self->pt_lid = _lwp_self(); 272 self->pt_lid = _lwp_self();
275} 273}
276 274
277static void 
278pthread__child_callback(void) 
279{ 
280 
281 /* 
282 * Clean up data structures that a forked child process might 
283 * trip over. Note that if threads have been created (causing 
284 * this handler to be registered) the standards say that the 
285 * child will trigger undefined behavior if it makes any 
286 * pthread_* calls (or any other calls that aren't 
287 * async-signal-safe), so we don't really have to clean up 
288 * much. Anything that permits some pthread_* calls to work is 
289 * merely being polite. 
290 */ 
291 pthread__started = 0; 
292} 
293 
294static void 
295pthread__start(void) 
296{ 
297 
298 /* 
299 * Per-process timers are cleared by fork(); despite the 
300 * various restrictions on fork() and threads, it's legal to 
301 * fork() before creating any threads. 
302 */ 
303 pthread_atfork(NULL, NULL, pthread__child_callback); 
304} 
305 
306 
307/* General-purpose thread data structure sanitization. */ 275/* General-purpose thread data structure sanitization. */
308/* ARGSUSED */ 276/* ARGSUSED */
309static void 277static void
310pthread__initthread(pthread_t t) 278pthread__initthread(pthread_t t)
311{ 279{
312 280
313 t->pt_self = t; 281 t->pt_self = t;
314 t->pt_magic = PT_MAGIC; 282 t->pt_magic = PT_MAGIC;
315 t->pt_willpark = 0; 283 t->pt_willpark = 0;
316 t->pt_unpark = 0; 284 t->pt_unpark = 0;
317 t->pt_nwaiters = 0; 285 t->pt_nwaiters = 0;
318 t->pt_sleepobj = NULL; 286 t->pt_sleepobj = NULL;
319 t->pt_signalled = 0; 287 t->pt_signalled = 0;
@@ -414,42 +382,35 @@ pthread_create(pthread_t *thread, const  @@ -414,42 +382,35 @@ pthread_create(pthread_t *thread, const
414 pthread_attr_t nattr; 382 pthread_attr_t nattr;
415 struct pthread_attr_private *p; 383 struct pthread_attr_private *p;
416 char * volatile name; 384 char * volatile name;
417 unsigned long flag; 385 unsigned long flag;
418 void *private_area; 386 void *private_area;
419 int ret; 387 int ret;
420 388
421 if (__predict_false(__uselibcstub)) { 389 if (__predict_false(__uselibcstub)) {
422 pthread__errorfunc(__FILE__, __LINE__, __func__, 390 pthread__errorfunc(__FILE__, __LINE__, __func__,
423 "pthread_create() requires linking with -lpthread"); 391 "pthread_create() requires linking with -lpthread");
424 return __libc_thr_create_stub(thread, attr, startfunc, arg); 392 return __libc_thr_create_stub(thread, attr, startfunc, arg);
425 } 393 }
426 394
427 /* 
428 * It's okay to check this without a lock because there can 
429 * only be one thread before it becomes true. 
430 */ 
431 if (pthread__started == 0) { 
432 pthread__start(); 
433 pthread__started = 1; 
434 } 
435 
436 if (attr == NULL) 395 if (attr == NULL)
437 nattr = pthread_default_attr; 396 nattr = pthread_default_attr;
438 else if (attr->pta_magic == PT_ATTR_MAGIC) 397 else if (attr->pta_magic == PT_ATTR_MAGIC)
439 nattr = *attr; 398 nattr = *attr;
440 else 399 else
441 return EINVAL; 400 return EINVAL;
442 401
 402 pthread__started = 1;
 403
443 /* Fetch misc. attributes from the attr structure. */ 404 /* Fetch misc. attributes from the attr structure. */
444 name = NULL; 405 name = NULL;
445 if ((p = nattr.pta_private) != NULL) 406 if ((p = nattr.pta_private) != NULL)
446 if (p->ptap_name[0] != '\0') 407 if (p->ptap_name[0] != '\0')
447 if ((name = strdup(p->ptap_name)) == NULL) 408 if ((name = strdup(p->ptap_name)) == NULL)
448 return ENOMEM; 409 return ENOMEM;
449 410
450 newthread = NULL; 411 newthread = NULL;
451 412
452 /* 413 /*
453 * Try to reclaim a dead thread. 414 * Try to reclaim a dead thread.
454 */ 415 */
455 if (!PTQ_EMPTY(&pthread__deadqueue)) { 416 if (!PTQ_EMPTY(&pthread__deadqueue)) {

cvs diff -r1.67 -r1.68 src/lib/libpthread/pthread_cond.c (expand / switch to unified diff)

--- src/lib/libpthread/pthread_cond.c 2020/01/29 15:07:46 1.67
+++ src/lib/libpthread/pthread_cond.c 2020/04/14 23:35:07 1.68
@@ -1,14 +1,14 @@ @@ -1,14 +1,14 @@
1/* $NetBSD: pthread_cond.c,v 1.67 2020/01/29 15:07:46 kamil Exp $ */ 1/* $NetBSD: pthread_cond.c,v 1.68 2020/04/14 23:35:07 joerg Exp $ */
2 2
3/*- 3/*-
4 * Copyright (c) 2001, 2006, 2007, 2008 The NetBSD Foundation, Inc. 4 * Copyright (c) 2001, 2006, 2007, 2008 The NetBSD Foundation, Inc.
5 * All rights reserved. 5 * All rights reserved.
6 * 6 *
7 * This code is derived from software contributed to The NetBSD Foundation 7 * This code is derived from software contributed to The NetBSD Foundation
8 * by Nathan J. Williams and Andrew Doran. 8 * by Nathan J. Williams and Andrew Doran.
9 * 9 *
10 * Redistribution and use in source and binary forms, with or without 10 * Redistribution and use in source and binary forms, with or without
11 * modification, are permitted provided that the following conditions 11 * modification, are permitted provided that the following conditions
12 * are met: 12 * are met:
13 * 1. Redistributions of source code must retain the above copyright 13 * 1. Redistributions of source code must retain the above copyright
14 * notice, this list of conditions and the following disclaimer. 14 * notice, this list of conditions and the following disclaimer.
@@ -36,44 +36,39 @@ @@ -36,44 +36,39 @@
36 * spinlock is present only to prevent libpthread causing the application 36 * spinlock is present only to prevent libpthread causing the application
37 * to crash or malfunction as a result of corrupted data structures, in 37 * to crash or malfunction as a result of corrupted data structures, in
38 * the event that the application is buggy. 38 * the event that the application is buggy.
39 * 39 *
40 * If there is contention on spinlock when real-time threads are in use, 40 * If there is contention on spinlock when real-time threads are in use,
41 * it could cause a deadlock due to priority inversion: the thread holding 41 * it could cause a deadlock due to priority inversion: the thread holding
42 * the spinlock may not get CPU time to make forward progress and release 42 * the spinlock may not get CPU time to make forward progress and release
43 * the spinlock to a higher priority thread that is waiting for it. 43 * the spinlock to a higher priority thread that is waiting for it.
44 * Contention on the spinlock will only occur with buggy applications, 44 * Contention on the spinlock will only occur with buggy applications,
45 * so at the time of writing it's not considered a major bug in libpthread. 45 * so at the time of writing it's not considered a major bug in libpthread.
46 */ 46 */
47 47
48#include <sys/cdefs.h> 48#include <sys/cdefs.h>
49__RCSID("$NetBSD: pthread_cond.c,v 1.67 2020/01/29 15:07:46 kamil Exp $"); 49__RCSID("$NetBSD: pthread_cond.c,v 1.68 2020/04/14 23:35:07 joerg Exp $");
50 50
51#include <stdlib.h> 51#include <stdlib.h>
52#include <errno.h> 52#include <errno.h>
53#include <sys/time.h> 53#include <sys/time.h>
54#include <sys/types.h> 54#include <sys/types.h>
55 55
56#include "pthread.h" 56#include "pthread.h"
57#include "pthread_int.h" 57#include "pthread_int.h"
58#include "reentrant.h" 58#include "reentrant.h"
59 59
60int _sys___nanosleep50(const struct timespec *, struct timespec *); 60int _sys___nanosleep50(const struct timespec *, struct timespec *);
61 61
62extern int pthread__started; 
63 
64static int pthread_cond_wait_nothread(pthread_t, pthread_mutex_t *, 
65 pthread_cond_t *, const struct timespec *); 
66 
67int _pthread_cond_has_waiters_np(pthread_cond_t *); 62int _pthread_cond_has_waiters_np(pthread_cond_t *);
68 63
69__weak_alias(pthread_cond_has_waiters_np,_pthread_cond_has_waiters_np) 64__weak_alias(pthread_cond_has_waiters_np,_pthread_cond_has_waiters_np)
70 65
71__strong_alias(__libc_cond_init,pthread_cond_init) 66__strong_alias(__libc_cond_init,pthread_cond_init)
72__strong_alias(__libc_cond_signal,pthread_cond_signal) 67__strong_alias(__libc_cond_signal,pthread_cond_signal)
73__strong_alias(__libc_cond_broadcast,pthread_cond_broadcast) 68__strong_alias(__libc_cond_broadcast,pthread_cond_broadcast)
74__strong_alias(__libc_cond_wait,pthread_cond_wait) 69__strong_alias(__libc_cond_wait,pthread_cond_wait)
75__strong_alias(__libc_cond_timedwait,pthread_cond_timedwait) 70__strong_alias(__libc_cond_timedwait,pthread_cond_timedwait)
76__strong_alias(__libc_cond_destroy,pthread_cond_destroy) 71__strong_alias(__libc_cond_destroy,pthread_cond_destroy)
77 72
78static clockid_t 73static clockid_t
79pthread_cond_getclock(const pthread_cond_t *cond) 74pthread_cond_getclock(const pthread_cond_t *cond)
@@ -139,30 +134,26 @@ pthread_cond_timedwait(pthread_cond_t *c @@ -139,30 +134,26 @@ pthread_cond_timedwait(pthread_cond_t *c
139 134
140 if (__predict_false(__uselibcstub)) 135 if (__predict_false(__uselibcstub))
141 return __libc_cond_timedwait_stub(cond, mutex, abstime); 136 return __libc_cond_timedwait_stub(cond, mutex, abstime);
142 137
143 pthread__error(EINVAL, "Invalid condition variable", 138 pthread__error(EINVAL, "Invalid condition variable",
144 cond->ptc_magic == _PT_COND_MAGIC); 139 cond->ptc_magic == _PT_COND_MAGIC);
145 pthread__error(EINVAL, "Invalid mutex", 140 pthread__error(EINVAL, "Invalid mutex",
146 mutex->ptm_magic == _PT_MUTEX_MAGIC); 141 mutex->ptm_magic == _PT_MUTEX_MAGIC);
147 pthread__error(EPERM, "Mutex not locked in condition wait", 142 pthread__error(EPERM, "Mutex not locked in condition wait",
148 mutex->ptm_owner != NULL); 143 mutex->ptm_owner != NULL);
149 144
150 self = pthread__self(); 145 self = pthread__self();
151 146
152 /* Just hang out for a while if threads aren't running yet. */ 
153 if (__predict_false(pthread__started == 0)) { 
154 return pthread_cond_wait_nothread(self, mutex, cond, abstime); 
155 } 
156 if (__predict_false(self->pt_cancel)) { 147 if (__predict_false(self->pt_cancel)) {
157 pthread__cancelled(); 148 pthread__cancelled();
158 } 149 }
159 150
160 /* Note this thread as waiting on the CV. */ 151 /* Note this thread as waiting on the CV. */
161 pthread__spinlock(self, &cond->ptc_lock); 152 pthread__spinlock(self, &cond->ptc_lock);
162 cond->ptc_mutex = mutex; 153 cond->ptc_mutex = mutex;
163 PTQ_INSERT_HEAD(&cond->ptc_waiters, self, pt_sleep); 154 PTQ_INSERT_HEAD(&cond->ptc_waiters, self, pt_sleep);
164 self->pt_sleepobj = cond; 155 self->pt_sleepobj = cond;
165 pthread__spinunlock(self, &cond->ptc_lock); 156 pthread__spinunlock(self, &cond->ptc_lock);
166 157
167 do { 158 do {
168 self->pt_willpark = 1; 159 self->pt_willpark = 1;
@@ -421,48 +412,13 @@ pthread_condattr_setpshared(pthread_cond @@ -421,48 +412,13 @@ pthread_condattr_setpshared(pthread_cond
421 412
422 pthread__error(EINVAL, "Invalid condition variable attribute", 413 pthread__error(EINVAL, "Invalid condition variable attribute",
423 attr->ptca_magic == _PT_CONDATTR_MAGIC); 414 attr->ptca_magic == _PT_CONDATTR_MAGIC);
424 415
425 switch(pshared) { 416 switch(pshared) {
426 case PTHREAD_PROCESS_PRIVATE: 417 case PTHREAD_PROCESS_PRIVATE:
427 return 0; 418 return 0;
428 case PTHREAD_PROCESS_SHARED: 419 case PTHREAD_PROCESS_SHARED:
429 return ENOSYS; 420 return ENOSYS;
430 } 421 }
431 return EINVAL; 422 return EINVAL;
432} 423}
433#endif 424#endif
434 
435/* Utility routine to hang out for a while if threads haven't started yet. */ 
436static int 
437pthread_cond_wait_nothread(pthread_t self, pthread_mutex_t *mutex, 
438 pthread_cond_t *cond, const struct timespec *abstime) 
439{ 
440 struct timespec now, diff; 
441 int retval; 
442 
443 if (abstime == NULL) { 
444 diff.tv_sec = 99999999; 
445 diff.tv_nsec = 0; 
446 } else { 
447 clockid_t clck = pthread_cond_getclock(cond); 
448 clock_gettime(clck, &now); 
449 if (timespeccmp(abstime, &now, <)) 
450 timespecclear(&diff); 
451 else 
452 timespecsub(abstime, &now, &diff); 
453 } 
454 
455 do { 
456 pthread__testcancel(self); 
457 pthread_mutex_unlock(mutex); 
458 retval = _sys___nanosleep50(&diff, NULL); 
459 pthread_mutex_lock(mutex); 
460 } while (abstime == NULL && retval == 0); 
461 pthread__testcancel(self); 
462 
463 if (retval == 0) 
464 return ETIMEDOUT; 
465 else 
466 /* spurious wakeup */ 
467 return 0; 
468}