Bug #14807
closed2.6.0-preview2 segfaults on OpenBSD due to missing pthread_condattr_init call
Description
r63238 refactored thread_pthread.c, and where there was previously a pthread_condattr_init call to initialize the pthread_condattr_t value, it removed the call and passed the pthread_condattr_t* directly to pthread_condattr_setclock without initializing the value by calling pthread_condattr_init first. On some operating systems that works, but it's not required to work, and it segfaults on OpenBSD because the pthread_condattr_t is not initialized.
The attached patch should fix the problem.
Files
Updated by normalperson (Eric Wong) over 6 years ago
Thanks, r63548
Btw, is PTHREAD_COND_INITIALIZER supported on OpenBSD?
Something like this:
--- a/thread_pthread.c
+++ b/thread_pthread.c
@@ -55,7 +55,7 @@ static struct {
#if defined(HAVE_PTHREAD_CONDATTR_SETCLOCK) && \
defined(CLOCK_REALTIME) && defined(CLOCK_MONOTONIC) && \
defined(HAVE_CLOCK_GETTIME)
-static pthread_condattr_t condattr_mono;
+static pthread_condattr_t condattr_mono = PTHREAD_COND_INITIALIZER;
static pthread_condattr_t *condattr_monotonic = &condattr_mono;
#else
static const void *const condattr_monotonic = NULL;
...And reverting your patch. Should save a few instructions at
startup since it avoids linkage and function call at startup.
Updated by jeremyevans0 (Jeremy Evans) over 6 years ago
normalperson (Eric Wong) wrote:
Btw, is PTHREAD_COND_INITIALIZER supported on OpenBSD?
It's defined but I don't think it would be usable:
/usr/include/pthread.h:#define PTHREAD_COND_INITIALIZER NULL
Updated by taca (Takahiro Kambe) about 6 years ago
Hi,
The similar problem occurs on NetBSD 8.0_STABLE. (And I belive it would be occur on 7.2.)
PTHREAD_COND_INITIALIZER
is for pthread_cond_t
not for pthread_condattr_t
.
So, initializing condattr_mono
(via condattr_monotonic
) with pthread_condattr_init()
is correct way to fix the problem as the attached patch.
Best regards.
P.S.
Oh, trunk was already applied the patch!
Updated by normalperson (Eric Wong) about 6 years ago
Right, already in trunk at r63548
And back to Jeremy's earlier comment:
It's defined but I don't think it would be usable:
/usr/include/pthread.h:#define PTHREAD_COND_INITIALIZER NULL
Without reading the OpenBSD pthreads library source code;
I suspect that it would be usable.
The condvar implementation can safely perform lazy-initialization
because it can rely on the underlying mutex for protection.
Updated by taca (Takahiro Kambe) about 6 years ago
Hi,
Wheather PTHREAD_COND_INITIALIZER
is work on OpenBSD or not, it should be used for initialize pthread_cond_t
variable and
it should not be used for initialize pthread_condattr_t
variable since they are diffrernet type and it cause compile error no NetBSD.
Updated by jeremyevans0 (Jeremy Evans) over 5 years ago
- Status changed from Open to Closed
Fixed by 832b601e49fd402ec7f30b36a95473131e93ae94. As taca explained, PTHREAD_COND_INITIALIZER should not be used for pthread_condattr_t.