From a7dd25be55a2f0e2e528cc9b1e320b0293821425 Mon Sep 17 00:00:00 2001 From: David Young Date: Wed, 5 Feb 2020 16:47:55 -0600 Subject: If H5_HAVE_THREADSAFE is not #defined, define nothing but a stub implementation of H5TS_thread_id(). --- src/H5TSprivate.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/H5TSprivate.h b/src/H5TSprivate.h index f22ed52..887f001 100644 --- a/src/H5TSprivate.h +++ b/src/H5TSprivate.h @@ -26,6 +26,7 @@ #ifndef H5TSprivate_H_ #define H5TSprivate_H_ +#ifdef H5_HAVE_THREADSAFE /* Public headers needed by this file */ #ifdef LATER #include "H5TSpublic.h" /*Public API prototypes */ @@ -128,5 +129,11 @@ H5_DLL H5TS_thread_t H5TS_create_thread(void *(*func)(void *), H5TS_attr_t * att } #endif /* c_plusplus || __cplusplus */ +#else /* H5_HAVE_THREADSAFE */ + +#define H5TS_thread_id() ((uint64_t)0) + +#endif /* H5_HAVE_THREADSAFE */ + #endif /* H5TSprivate_H_ */ -- cgit v0.12 From 8aa74137dd79f07ac2fecafbd7dbcc8e1b537af2 Mon Sep 17 00:00:00 2001 From: David Young Date: Wed, 5 Feb 2020 19:31:59 -0600 Subject: Use a naked pthread_self() call in the HDF5 thread wrappers. --- src/H5TS.c | 4 ++-- src/H5private.h | 4 ---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/H5TS.c b/src/H5TS.c index 7a801e2..cc55810 100644 --- a/src/H5TS.c +++ b/src/H5TS.c @@ -262,7 +262,7 @@ H5TS_mutex_lock(H5TS_mutex_t *mutex) if (ret_value) return ret_value; - if(mutex->lock_count && pthread_equal(HDpthread_self(), mutex->owner_thread)) { + if(mutex->lock_count && pthread_equal(pthread_self(), mutex->owner_thread)) { /* already owned by self - increment count */ mutex->lock_count++; } else { @@ -271,7 +271,7 @@ H5TS_mutex_lock(H5TS_mutex_t *mutex) pthread_cond_wait(&mutex->cond_var, &mutex->atomic_lock); /* After we've received the signal, take ownership of the mutex */ - mutex->owner_thread = HDpthread_self(); + mutex->owner_thread = pthread_self(); mutex->lock_count = 1; } diff --git a/src/H5private.h b/src/H5private.h index e7626ac..ff5e195 100644 --- a/src/H5private.h +++ b/src/H5private.h @@ -1567,10 +1567,6 @@ extern char *strdup(const char *s); #define HDstrdup(S) strdup(S) #endif /* HDstrdup */ -#ifndef HDpthread_self - #define HDpthread_self() pthread_self() -#endif /* HDpthread_self */ - /* Macro for "stringizing" an integer in the C preprocessor (use H5_TOSTRING) */ /* (use H5_TOSTRING, H5_STRINGIZE is just part of the implementation) */ #define H5_STRINGIZE(x) #x -- cgit v0.12 From 2ba97e40cdb0950be7ea4858607e42118d9dce3e Mon Sep 17 00:00:00 2001 From: David Young Date: Fri, 7 Feb 2020 14:57:46 -0600 Subject: Make sure that H5TS_thread_id() is available as either a function or a macro in all configurations. Previously it was neither declared nor defined in --disable-threadsafety builds. The compiler's warning got lost in the noise---I first saw the issue because my -Werror branch stopped compiling cold---and the tests still linked and ran. --- src/H5TS.c | 9 --------- src/H5private.h | 8 +++++--- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/H5TS.c b/src/H5TS.c index cc55810..b0cef7f 100644 --- a/src/H5TS.c +++ b/src/H5TS.c @@ -639,13 +639,4 @@ H5TS_create_thread(void *(*func)(void *), H5TS_attr_t *attr, void *udata) } /* H5TS_create_thread */ -#else /* H5_HAVE_THREADSAFE */ - -uint64_t -H5TS_thread_id(void) -{ - return 0; -} - #endif /* H5_HAVE_THREADSAFE */ - diff --git a/src/H5private.h b/src/H5private.h index ff5e195..ff6cc77 100644 --- a/src/H5private.h +++ b/src/H5private.h @@ -1915,12 +1915,14 @@ H5_DLL double H5_trace(const double *calltime, const char *func, const char *typ /* global library version information string */ extern char H5_lib_vers_info_g[]; +/* Include required thread-safety header, always, for the H5TS_thread_id() + * definition. + */ +#include "H5TSprivate.h" + /* Lock headers */ #ifdef H5_HAVE_THREADSAFE -/* Include required thread-safety header */ -#include "H5TSprivate.h" - /* replacement structure for original global variable */ typedef struct H5_api_struct { H5TS_mutex_t init_lock; /* API entrance mutex */ -- cgit v0.12 From d64afcd9d0ea3be06bb4fefd2d4c1ebd291fef84 Mon Sep 17 00:00:00 2001 From: David Young Date: Fri, 7 Feb 2020 16:34:01 -0600 Subject: Follow HDF5 conventions. --- src/H5TS.c | 119 +++++++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 89 insertions(+), 30 deletions(-) diff --git a/src/H5TS.c b/src/H5TS.c index b0cef7f..b67391c 100644 --- a/src/H5TS.c +++ b/src/H5TS.c @@ -39,26 +39,26 @@ H5TS_key_t H5TS_cancel_key_g; #ifndef H5_HAVE_WIN_THREADS -/* An h5_tid_t is a record of a thread identifier that is +/* An H5TS_tid_t is a record of a thread identifier that is * available for reuse. */ struct _tid; -typedef struct _tid h5_tid_t; +typedef struct _tid H5TS_tid_t; struct _tid { - h5_tid_t *next; + H5TS_tid_t *next; uint64_t id; }; /* Pointer to first free thread ID record or NULL. */ -static h5_tid_t *tid_next_free = NULL; -static uint64_t tid_next_id = 0; +static H5TS_tid_t *H5TS_tid_next_free = NULL; +static uint64_t H5TS_tid_next_id = 0; -/* Mutual exclusion for access to tid_next_free and tid_next_id. */ -static pthread_mutex_t tid_mtx; +/* Mutual exclusion for access to H5TS_tid_next_free and H5TS_tid_next_id. */ +static pthread_mutex_t H5TS_tid_mtx; /* Key for thread-local storage of the thread ID. */ -static H5TS_key_t tid_key; +static H5TS_key_t H5TS_tid_key; #endif /* H5_HAVE_WIN_THREADS */ @@ -93,29 +93,61 @@ H5TS_key_destructor(void *key_val) #ifndef H5_HAVE_WIN_THREADS -/* When a thread shuts down, put its ID record on the free list. */ +/*-------------------------------------------------------------------------- + * NAME + * H5TS_tid_destructor + * + * USAGE + * H5TS_tid_destructor() + * + * RETURNS + * + * DESCRIPTION + * When a thread shuts down, put its ID record on the free list. + * + * PROGRAMMER: Rusty Shackleford + * February 7, 2020 + * + *-------------------------------------------------------------------------- + */ static void -tid_destructor(void *_v) +H5TS_tid_destructor(void *_v) { - h5_tid_t *tid = _v; + H5TS_tid_t *tid = _v; if (tid == NULL) return; /* XXX I can use mutexes in destructors, right? */ /* TBD use an atomic CAS */ - pthread_mutex_lock(&tid_mtx); - tid->next = tid_next_free; - tid_next_free = tid; - pthread_mutex_unlock(&tid_mtx); + pthread_mutex_lock(&H5TS_tid_mtx); + tid->next = H5TS_tid_next_free; + H5TS_tid_next_free = tid; + pthread_mutex_unlock(&H5TS_tid_mtx); } -/* Initialize for integer thread identifiers. */ +/*-------------------------------------------------------------------------- + * NAME + * H5TS_tid_init + * + * USAGE + * H5TS_tid_init() + * + * RETURNS + * + * DESCRIPTION + * Initialize for integer thread identifiers. + * + * PROGRAMMER: Dale Alvin Gribble + * February 7, 2020 + * + *-------------------------------------------------------------------------- + */ static void -tid_init(void) +H5TS_tid_init(void) { - pthread_mutex_init(&tid_mtx, NULL); - pthread_key_create(&tid_key, tid_destructor); + pthread_mutex_init(&H5TS_tid_mtx, NULL); + pthread_key_create(&H5TS_tid_key, H5TS_tid_destructor); } /* Return an integer identifier, ID, for the current thread satisfying the @@ -129,11 +161,38 @@ tid_init(void) * ID 0 is reserved. H5TS_thread_id() returns 0 if the library was not built * with thread safety or if an error prevents it from assigning an ID. */ +/*-------------------------------------------------------------------------- + * NAME + * H5TS_thread_id + * + * USAGE + * uint64_t id = H5TS_thread_id() + * + * RETURNS + * Return an integer identifier, ID, for the current thread. + * + * DESCRIPTION + * The ID satisfies the following properties: + * + * 1 1 <= ID <= UINT64_MAX + * 2 ID is constant over the thread's lifetime. + * 3 No two threads share an ID during their lifetimes. + * 4 A thread's ID is available for reuse as soon as it is joined. + * + * ID 0 is reserved. H5TS_thread_id() returns 0 if the library was not + * built with thread safety or if an error prevents it from assigning an + * ID. + * + * PROGRAMMER: Rusty Shackleford + * February 7, 2020 + * + *-------------------------------------------------------------------------- + */ uint64_t H5TS_thread_id(void) { - h5_tid_t *tid = pthread_getspecific(tid_key); - h5_tid_t proto_tid; + H5TS_tid_t *tid = pthread_getspecific(H5TS_tid_key); + H5TS_tid_t proto_tid; /* An ID is already assigned. */ if (tid != NULL) @@ -146,14 +205,14 @@ H5TS_thread_id(void) * point `tid` at `proto_tid` if we need to allocate some * memory. */ - pthread_mutex_lock(&tid_mtx); - if ((tid = tid_next_free) != NULL) - tid_next_free = tid->next; - else if (tid_next_id != UINT64_MAX) { + pthread_mutex_lock(&H5TS_tid_mtx); + if ((tid = H5TS_tid_next_free) != NULL) + H5TS_tid_next_free = tid->next; + else if (H5TS_tid_next_id != UINT64_MAX) { tid = &proto_tid; - tid->id = ++tid_next_id; + tid->id = ++H5TS_tid_next_id; } - pthread_mutex_unlock(&tid_mtx); + pthread_mutex_unlock(&H5TS_tid_mtx); /* If a prototype ID record was established, copy it to the heap. */ if (tid == &proto_tid) { @@ -168,8 +227,8 @@ H5TS_thread_id(void) * to it. */ tid->next = NULL; - if (pthread_setspecific(tid_key, tid) != 0) { - tid_destructor(tid); + if (pthread_setspecific(H5TS_tid_key, tid) != 0) { + H5TS_tid_destructor(tid); return 0; } @@ -213,7 +272,7 @@ H5TS_pthread_first_thread_init(void) H5_g.init_lock.lock_count = 0; /* Initialize integer thread identifiers. */ - tid_init(); + H5TS_tid_init(); /* initialize key for thread-specific error stacks */ pthread_key_create(&H5TS_errstk_key_g, H5TS_key_destructor); -- cgit v0.12 From c97981da3620cff7f789516dd9dff5ffde86d6d8 Mon Sep 17 00:00:00 2001 From: David Young Date: Mon, 10 Feb 2020 17:24:04 -0600 Subject: Remove tongue-in-cheek credit for Rusty Shackleford and Dale Alvin Gribble. Delete the comment questioning whether pthread_mutex_lock is allowed in a key destructor, since pthread_key_create(3) provides the answer: There is no notion of a destructor-safe function. If an application does not call pthread_exit() from a signal handler, or if it blocks any signal whose handler may call pthread_exit() while calling async-unsafe functions, all functions may be safely called from destructors. Delete redundant comment. --- src/H5TS.c | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/src/H5TS.c b/src/H5TS.c index b67391c..98eba41 100644 --- a/src/H5TS.c +++ b/src/H5TS.c @@ -105,9 +105,6 @@ H5TS_key_destructor(void *key_val) * DESCRIPTION * When a thread shuts down, put its ID record on the free list. * - * PROGRAMMER: Rusty Shackleford - * February 7, 2020 - * *-------------------------------------------------------------------------- */ static void @@ -118,7 +115,6 @@ H5TS_tid_destructor(void *_v) if (tid == NULL) return; - /* XXX I can use mutexes in destructors, right? */ /* TBD use an atomic CAS */ pthread_mutex_lock(&H5TS_tid_mtx); tid->next = H5TS_tid_next_free; @@ -138,9 +134,6 @@ H5TS_tid_destructor(void *_v) * DESCRIPTION * Initialize for integer thread identifiers. * - * PROGRAMMER: Dale Alvin Gribble - * February 7, 2020 - * *-------------------------------------------------------------------------- */ static void @@ -150,17 +143,6 @@ H5TS_tid_init(void) pthread_key_create(&H5TS_tid_key, H5TS_tid_destructor); } -/* Return an integer identifier, ID, for the current thread satisfying the - * following properties: - * - * 1 1 <= ID <= UINT64_MAX - * 2 ID is constant over the thread's lifetime. - * 3 No two threads share an ID during their lifetimes. - * 4 A thread's ID is available for reuse as soon as it is joined. - * - * ID 0 is reserved. H5TS_thread_id() returns 0 if the library was not built - * with thread safety or if an error prevents it from assigning an ID. - */ /*-------------------------------------------------------------------------- * NAME * H5TS_thread_id @@ -183,9 +165,6 @@ H5TS_tid_init(void) * built with thread safety or if an error prevents it from assigning an * ID. * - * PROGRAMMER: Rusty Shackleford - * February 7, 2020 - * *-------------------------------------------------------------------------- */ uint64_t -- cgit v0.12 From c0fa07fd9fc1c4b0aab116c6c278142384a440b9 Mon Sep 17 00:00:00 2001 From: David Young Date: Mon, 3 Feb 2020 16:33:28 -0600 Subject: src/H5Eint.c: #include H5TSprivate.h for H5TS_thread_id() definitions. test/thread_id.c: move threads_failure() inside #ifdefs. --- src/H5Eint.c | 1 + test/thread_id.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/H5Eint.c b/src/H5Eint.c index 7e144c8..735f0f2 100644 --- a/src/H5Eint.c +++ b/src/H5Eint.c @@ -37,6 +37,7 @@ #include "H5Epkg.h" /* Error handling */ #include "H5Iprivate.h" /* IDs */ #include "H5MMprivate.h" /* Memory management */ +#include "H5TSprivate.h" /* Thread stuff */ /****************/ diff --git a/test/thread_id.c b/test/thread_id.c index 75ffe17..8f6e2d5 100644 --- a/test/thread_id.c +++ b/test/thread_id.c @@ -26,13 +26,13 @@ */ #include "testhdf5.h" +#if defined(H5_HAVE_THREADSAFE) && !defined(H5_HAVE_WIN_THREADS) + #define threads_failure(_call, _result) do { \ errx(EXIT_FAILURE, "%s.%d: " #_call ": %s", __func__, \ __LINE__, strerror(_result)); \ } while (false) -#if defined(H5_HAVE_THREADSAFE) && !defined(H5_HAVE_WIN_THREADS) - #define NTHREADS 5 static volatile bool failed = false; -- cgit v0.12 From efca0c7f5644b4bb6bcb13b9f869087db8a78c83 Mon Sep 17 00:00:00 2001 From: David Young Date: Thu, 6 Feb 2020 10:43:37 -0600 Subject: Oops, the test has to return success in the unimplemented case. --- test/thread_id.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/thread_id.c b/test/thread_id.c index 8f6e2d5..9b289fa 100644 --- a/test/thread_id.c +++ b/test/thread_id.c @@ -156,8 +156,7 @@ main(void) int main(void) { - errx(EXIT_FAILURE, "not implemented in this configuration."); + HDfprintf(stderr, "not implemented in this configuration.\n"); + return EXIT_SUCCESS; } - #endif /*H5_HAVE_THREADSAFE && !H5_HAVE_WIN_THREADS*/ - -- cgit v0.12 From d7412b7e88f02ea6784ba5ba97568137ab38c6a6 Mon Sep 17 00:00:00 2001 From: David Young Date: Fri, 7 Feb 2020 14:45:08 -0600 Subject: Provide local copies of err(3)- and errx(3)-alike functions for Visual Studio compatibility. --- test/thread_id.c | 44 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/test/thread_id.c b/test/thread_id.c index 9b289fa..21be12d 100644 --- a/test/thread_id.c +++ b/test/thread_id.c @@ -18,7 +18,6 @@ * 3 No two threads share an ID during their lifetimes. * 4 A thread's ID is available for reuse as soon as it is joined. */ -#include /* * Include required headers. This file tests internal library functions, @@ -26,10 +25,41 @@ */ #include "testhdf5.h" +static void my_errx(int, const char *, ...) H5_ATTR_FORMAT(printf, 2, 3); + +static void +my_errx(int code, const char *fmt, ...) +{ + va_list ap; + + (void)fprintf(stderr, "thread_id: "); + va_start(ap, fmt); + (void)vfprintf(stderr, fmt, ap); + va_end(ap); + (void)fputc('\n', stderr); + exit(code); +} + #if defined(H5_HAVE_THREADSAFE) && !defined(H5_HAVE_WIN_THREADS) +static void my_err(int, const char *, ...) H5_ATTR_FORMAT(printf, 2, 3); + +static void +my_err(int code, const char *fmt, ...) +{ + va_list ap; + int errno_copy = errno; + + (void)fprintf(stderr, "thread_id: "); + va_start(ap, fmt); + (void)vfprintf(stderr, fmt, ap); + va_end(ap); + (void)fprintf(stderr, ": %s\n", strerror(errno_copy)); + exit(code); +} + #define threads_failure(_call, _result) do { \ - errx(EXIT_FAILURE, "%s.%d: " #_call ": %s", __func__, \ + my_errx(EXIT_FAILURE, "%s.%d: " #_call ": %s", __func__, \ __LINE__, strerror(_result)); \ } while (false) @@ -52,13 +82,13 @@ atomic_printf(const char *fmt, ...) va_end(ap); if (nprinted == -1) - err(EXIT_FAILURE, "%s.%d: vsnprintf", __func__, __LINE__); + my_err(EXIT_FAILURE, "%s.%d: vsnprintf", __func__, __LINE__); else if (nprinted >= (ssize_t)sizeof(buf)) - errx(EXIT_FAILURE, "%s.%d: vsnprintf overflowed", __func__, __LINE__); + my_errx(EXIT_FAILURE, "%s.%d: vsnprintf overflowed", __func__, __LINE__); nwritten = write(STDOUT_FILENO, buf, (size_t)nprinted); if (nwritten < nprinted) { - errx(EXIT_FAILURE, "%s.%d: write error or short write", + my_errx(EXIT_FAILURE, "%s.%d: write error or short write", __func__, __LINE__); } } @@ -114,7 +144,7 @@ main(void) * mutex, etc. */ if (H5open() != SUCCEED) - errx(EXIT_FAILURE, "%s.%d: H5open failed", __func__, __LINE__); + my_errx(EXIT_FAILURE, "%s.%d: H5open failed", __func__, __LINE__); if ((rc = pthread_mutex_init(&used_lock, NULL)) == -1) threads_failure(pthread_mutex_init, rc); @@ -144,7 +174,7 @@ main(void) for (i = 0; i < NTHREADS; i++) { if (!used[i]) // access synchronized by thread create/join - errx(EXIT_FAILURE, "thread ID %d did not run.", i + 1); + my_errx(EXIT_FAILURE, "thread ID %d did not run.", i + 1); } } if ((rc = pthread_barrier_destroy(&barrier)) != 0) -- cgit v0.12 From 0952346c795508371c52240568957d1d5343318d Mon Sep 17 00:00:00 2001 From: David Young Date: Wed, 12 Feb 2020 14:07:47 -0600 Subject: Use HD prefix. --- test/thread_id.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/test/thread_id.c b/test/thread_id.c index 21be12d..af36625 100644 --- a/test/thread_id.c +++ b/test/thread_id.c @@ -32,12 +32,12 @@ my_errx(int code, const char *fmt, ...) { va_list ap; - (void)fprintf(stderr, "thread_id: "); - va_start(ap, fmt); - (void)vfprintf(stderr, fmt, ap); - va_end(ap); - (void)fputc('\n', stderr); - exit(code); + (void)HDfprintf(stderr, "thread_id: "); + HDva_start(ap, fmt); + (void)HDvfprintf(stderr, fmt, ap); + HDva_end(ap); + (void)HDfputc('\n', stderr); + HDexit(code); } #if defined(H5_HAVE_THREADSAFE) && !defined(H5_HAVE_WIN_THREADS) @@ -50,17 +50,17 @@ my_err(int code, const char *fmt, ...) va_list ap; int errno_copy = errno; - (void)fprintf(stderr, "thread_id: "); - va_start(ap, fmt); - (void)vfprintf(stderr, fmt, ap); - va_end(ap); - (void)fprintf(stderr, ": %s\n", strerror(errno_copy)); - exit(code); + (void)HDfprintf(stderr, "thread_id: "); + HDva_start(ap, fmt); + (void)HDvfprintf(stderr, fmt, ap); + HDva_end(ap); + (void)HDfprintf(stderr, ": %s\n", HDstrerror(errno_copy)); + HDexit(code); } #define threads_failure(_call, _result) do { \ my_errx(EXIT_FAILURE, "%s.%d: " #_call ": %s", __func__, \ - __LINE__, strerror(_result)); \ + __LINE__, HDstrerror(_result)); \ } while (false) #define NTHREADS 5 -- cgit v0.12 From 4d6c96bdf572ed931560d5a525e133a246beea31 Mon Sep 17 00:00:00 2001 From: David Young Date: Wed, 26 Feb 2020 20:38:57 -0600 Subject: Implement pthread_barrier(3) for Darwin using a counter, condition variable, and mutex. Untested. --- test/thread_id.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/test/thread_id.c b/test/thread_id.c index af36625..4637d9c 100644 --- a/test/thread_id.c +++ b/test/thread_id.c @@ -42,6 +42,129 @@ my_errx(int code, const char *fmt, ...) #if defined(H5_HAVE_THREADSAFE) && !defined(H5_HAVE_WIN_THREADS) +#if defined(H5_HAVE_DARWIN) + +typedef struct _pthread_barrierattr { + uint8_t unused; +} pthread_barrierattr_t; + +typedef struct _pthread_barrier { + uint32_t magic; + unsigned int count; + uint64_t nentered; + pthread_cond_t cv; + pthread_mutex_t mtx; +} pthread_barrier_t; + +int pthread_barrier_init(pthread_barrier_t *, const pthread_barrierattr_t *, + unsigned int); +int pthread_barrier_wait(pthread_barrier_t *); +int pthread_barrier_destroy(pthread_barrier_t *); + +static const uint32_t barrier_magic = 0xf00dd00f; + +int +pthread_barrier_init(pthread_barrier_t *barrier, + const pthread_barrierattr_t *attr, unsigned int count) +{ + int rc; + + if (count == 0) + return EINVAL; + + if (attr != NULL) + return EINVAL; + + memset(barrier, 0, sizeof(*barrier)); + + barrier->count = count; + + if ((rc = pthread_cond_init(&barrier->cv, NULL)) != 0) + return rc; + + if ((rc = pthread_mutex_init(&barrier->mtx, NULL)) != 0) { + (void)pthread_cond_destroy(&barrier->cv); + return rc; + } + + barrier->magic = barrier_magic; + + return 0; +} + +static void +barrier_lock(pthread_barrier_t *barrier) +{ + int rc; + + if ((rc = pthread_mutex_lock(&barrier->mtx)) != 0) { + my_errx(exit_failure, "%s: pthread_mutex_lock: %s", __func__, + strerror(rc)); + } +} + +static void +barrier_unlock(pthread_barrier_t *barrier) +{ + int rc; + + if ((rc = pthread_mutex_unlock(&barrier->mtx)) != 0) { + my_errx(exit_failure, "%s: pthread_mutex_unlock: %s", __func__, + strerror(rc)); + } +} + +int +pthread_barrier_destroy(pthread_barrier_t *barrier) +{ + int rc; + + barrier_lock(barrier); + if (barrier->magic != barrier_magic) + rc = EINVAL; + else if (barrier->count != 0) + rc = EBUSY; + else { + rc = 0; + barrier->magic = ~barrier->magic; + } + barrier_unlock(barrier); + + if (rc != 0) + return rc; + + (void)pthread_cond_destroy(&barrier->cv); + (void)pthread_mutex_destroy(&barrier->mtx); + + return 0; +} + +int +pthread_barrier_wait(pthread_barrier_t *barrier) +{ + int rc; + + if (barrier == NULL) + return EINVAL; + + barrier_lock(barrier); + if (barrier->magic != barrier_magic) { + rc = EINVAL; + goto out; + } + barrier->nentered++; + while (barrier->nentered % barrier->count != 0) { + if ((rc = pthread_cond_wait(&barrier->cv, &barrier->mtx)) != 0) + goto out; + } + rc = pthread_cond_broadcast(&barrier->cv); +out: + barrier_unlock(barrier); + return rc; +} + +#endif /* H5_HAVE_DARWIN */ + static void my_err(int, const char *, ...) H5_ATTR_FORMAT(printf, 2, 3); static void -- cgit v0.12 From 99034ad9ad405a57f606817cb7083e809915a28c Mon Sep 17 00:00:00 2001 From: David Young Date: Thu, 27 Feb 2020 10:16:49 -0600 Subject: s/exit_failure/EXIT_FAILURE/g --- test/thread_id.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/thread_id.c b/test/thread_id.c index 4637d9c..84676bf 100644 --- a/test/thread_id.c +++ b/test/thread_id.c @@ -98,7 +98,7 @@ barrier_lock(pthread_barrier_t *barrier) int rc; if ((rc = pthread_mutex_lock(&barrier->mtx)) != 0) { - my_errx(exit_failure, "%s: pthread_mutex_lock: %s", __func__, + my_errx(EXIT_FAILURE, "%s: pthread_mutex_lock: %s", __func__, strerror(rc)); } } @@ -109,7 +109,7 @@ barrier_unlock(pthread_barrier_t *barrier) int rc; if ((rc = pthread_mutex_unlock(&barrier->mtx)) != 0) { - my_errx(exit_failure, "%s: pthread_mutex_unlock: %s", __func__, + my_errx(EXIT_FAILURE, "%s: pthread_mutex_unlock: %s", __func__, strerror(rc)); } } -- cgit v0.12 From e5f459d86d7f566fb57c324305c7cb365ac5ad08 Mon Sep 17 00:00:00 2001 From: David Young Date: Thu, 27 Feb 2020 10:17:24 -0600 Subject: Test the right condition for the EBUSY return in pthread_barrier_destroy(). --- test/thread_id.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/thread_id.c b/test/thread_id.c index 84676bf..49e3710 100644 --- a/test/thread_id.c +++ b/test/thread_id.c @@ -122,7 +122,7 @@ pthread_barrier_destroy(pthread_barrier_t *barrier) barrier_lock(barrier); if (barrier->magic != barrier_magic) rc = EINVAL; - else if (barrier->count != 0) + else if (barrier->nentered % barrier->count != 0) rc = EBUSY; else { rc = 0; -- cgit v0.12 From c149f4ca16db6ae9538490041f3145d50585e65b Mon Sep 17 00:00:00 2001 From: David Young Date: Thu, 27 Feb 2020 11:27:45 -0600 Subject: The first implementation seemed to allow for the possibility that a thread could block at the barrier, wake and exit the barrier, re-acquire the barrier lock and increase `nentered` before the other blocked threads woke and checked `nentered % count == 0`. Then the other blocked threads would check `nentered % count == 0` and, finding it false, go back to sleep in the barrier. This new implementation waits for a looser condition to obtain so that threads don't go back to sleep in the barrier. --- test/thread_id.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/thread_id.c b/test/thread_id.c index 49e3710..e3cf42e 100644 --- a/test/thread_id.c +++ b/test/thread_id.c @@ -143,6 +143,7 @@ int pthread_barrier_wait(pthread_barrier_t *barrier) { int rc; + uint64_t threshold; if (barrier == NULL) return EINVAL; @@ -152,8 +153,14 @@ pthread_barrier_wait(pthread_barrier_t *barrier) rc = EINVAL; goto out; } + /* Compute the release `threshold`. All threads entering with count = 5 + * and `nentered` in [0, 4] should be released once `nentered` reaches 5: + * call 5 the release `threshold`. All threads entering with count = 5 + * and `nentered` in [5, 9] should be released once `nentered` reaches 10. + */ + threshold = (barrier->nentered / barrier->count + 1) * barrier->count; barrier->nentered++; - while (barrier->nentered % barrier->count != 0) { + while (barrier->nentered < threshold) { if ((rc = pthread_cond_wait(&barrier->cv, &barrier->mtx)) != 0) goto out; } -- cgit v0.12 From f0485413e02ebf1117e5b1725f32534e7e26b622 Mon Sep 17 00:00:00 2001 From: David Young Date: Thu, 27 Feb 2020 16:14:44 -0600 Subject: Complete the comment on thread_main(), explaining why the barrier is used. --- test/thread_id.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/thread_id.c b/test/thread_id.c index e3cf42e..818ab4a 100644 --- a/test/thread_id.c +++ b/test/thread_id.c @@ -226,7 +226,15 @@ atomic_printf(const char *fmt, ...) /* Each thread runs this routine. The routine fetches the current * thread's ID, makes sure that it is in the expected range, makes * sure that in this round of testing, no two threads shared the - * same ID, + * same ID, and checks that each thread's ID is constant over its lifetime. + * + * main() checks that every ID in [1, NTHREADS] is used in each round + * of testing. All NTHREADS threads synchronize on a barrier after each + * has fetched its ID. The barrier guarantees that all threads' lifetimes + * overlap at least momentarily, so the IDs will be unique, and there + * will be NTHREADS of them. Further, since thread IDs are assigned + * starting with 1, and the number of threads with IDs alive never exceeds + * NTHREADS, the least ID has to be 1 and the greatest, NTHREADS. */ static void * thread_main(void H5_ATTR_UNUSED *arg) -- cgit v0.12