WIP on updating thread-related code in the library.

Includes, but not limited to:
- Bring the recursive readers/writer lock (RRWL) from the 1.12 branch to develop
- Pulls together all the thread-related code into the H5TS package, removing
    sprawl and providing private API calls for other packages to use for e.g.
    acquiring thread-local variables, etc.
- Breaks H5TS package into several source files, instead of a single one, with
    large ifdef's.
- Added FUNC_ENTER_API_REENTER / FUNC_LEAVE_API_REENTER macros around all
    "developer" callback routines (e.g., the routines in H5VLdevelop.h,
    H5FDdevelop.h, etc.) that can be used for pass-through VOL connectors,
    stackable VFDs, etc., so they can have different thread-reentry behavior,
    like not re-acquiring the global lock.
- Moved all the thread-related tests into one test binary (test/ttsafe), with
    a "typical" use of test testhdf5.h macros.  This now includes the previously
    standalone thread_id test, and the tests for the RRWL.
- Added API context push/pop to tests that directly called routines that use
    FUNC_ENTER_API_REENTER / FUNC_LEAVE_API_REENTER.

Planned:
- Remove Mercury threading package, moving all usage back to single, HDF5-based
    thread-related wrappers, etc.
- Remove requirement for PTHREAD_MUTEX_ADAPTIVE_NP, pthread_condattr_setclock(),
    and CLOCK_MONOTONIC_COARSE, as they are built into the Mercury thread-
    related wrappers, but we don't need them.

Not related to threading, but also in this branch currently:
- Adds support for detecting and using the __builtin_expect() compiler hint,
    to let the compiler know branches that are likely/unlikely to be taken.
    Wrapped inside new H5_LIKELY/H5_UNLIKELY macros.

Signed-off-by: Quincey Koziol <[email protected]>

CMakeLists.txt

@@ -796,14 +796,6 @@ if (HDF5_ENABLE_SUBFILING_VFD)
   set (CMAKE_EXTRA_INCLUDE_FILES pthread.h)
   set (CMAKE_REQUIRED_LIBRARIES ${CMAKE_THREAD_LIBS_INIT})
 
-  check_type_size(PTHREAD_MUTEX_ADAPTIVE_NP PTHREAD_MUTEX_ADAPTIVE_NP_SIZE)
-  if (HAVE_PTHREAD_MUTEX_ADAPTIVE_NP_SIZE)
-    set (${HDF_PREFIX}_HAVE_PTHREAD_MUTEX_ADAPTIVE_NP 1)
-  endif ()
-
-  check_symbol_exists(pthread_condattr_setclock pthread.h
-    ${HDF_PREFIX}_HAVE_PTHREAD_CONDATTR_SETCLOCK)
-
   unset (CMAKE_EXTRA_INCLUDE_FILES)
   unset (CMAKE_REQUIRED_LIBRARIES)
 endif()

bin/trace

@@ -509,7 +509,7 @@ for $file (@ARGV) {
   $file_args = 0;
 
   # Ignore some files that do not need tracing macros
-  unless ($file eq "H5FDmulti.c" or $file eq "src/H5FDmulti.c" or $file eq "H5FDstdio.c" or $file eq "src/H5FDstdio.c" or $file eq "src/H5TS.c" or $file eq "src/H5FDperform.c") {
+  unless ($file eq "H5FDmulti.c" or $file eq "src/H5FDmulti.c" or $file eq "H5FDstdio.c" or $file eq "src/H5FDstdio.c" or $file eq "src/H5TS.c" or $file eq "src/H5TSmutex.c" or $file eq "src/H5FDperform.c") {
 
     # Snarf up the entire file
     open SOURCE, $file or die "$file: $!\n";

config/clang-warnings/error-general

@@ -57,8 +57,6 @@
 # Here is a list of tests and examples that have issues with the stricter warnings as error
 #
 # NOTE: Test files are not compatible with these warnings as errors
-#     thread_id.c,
-#       -Werror=unused-function
 #     dsets.c
 #       -Werror=unused-parameter
 #

config/cmake/ConfigureChecks.cmake

@@ -433,6 +433,8 @@ endif ()
 if (MINGW OR NOT WINDOWS)
   foreach (other_test
       HAVE_ATTRIBUTE
+      HAVE_BUILTIN_EXPECT
+      PTHREAD_BARRIER
       SYSTEM_SCOPE_THREADS
       HAVE_SOCKLEN_T
   )
@@ -642,12 +644,6 @@ if (MINGW OR NOT WINDOWS)
   endif ()
 endif ()
 
-# Check for clock_gettime() CLOCK_MONOTONIC_COARSE
-set (CMAKE_EXTRA_INCLUDE_FILES time.h)
-check_type_size(CLOCK_MONOTONIC_COARSE CLOCK_MONOTONIC_COARSE_SIZE)
-if (HAVE_CLOCK_MONOTONIC_COARSE_SIZE)
-  set (${HDF_PREFIX}_HAVE_CLOCK_MONOTONIC_COARSE 1)
-endif ()
 unset (CMAKE_EXTRA_INCLUDE_FILES)
 
 #-----------------------------------------------------------------------------

config/cmake/H5pubconf.h.in

@@ -107,9 +107,6 @@
 /* Define to 1 if you have the `clock_gettime' function. */
 #cmakedefine H5_HAVE_CLOCK_GETTIME @H5_HAVE_CLOCK_GETTIME@
 
-/* Define to 1 if CLOCK_MONOTONIC_COARSE is available */
-#cmakedefine H5_HAVE_CLOCK_MONOTONIC_COARSE @H5_HAVE_CLOCK_MONOTONIC_COARSE@
-
 /* Define if the function stack tracing code is to be compiled in */
 #cmakedefine H5_HAVE_CODESTACK @H5_HAVE_CODESTACK@
 
@@ -261,11 +258,8 @@
 /* Define to 1 if you have the <pthread.h> header file. */
 #cmakedefine H5_HAVE_PTHREAD_H @H5_HAVE_PTHREAD_H@
 
-/* Define to 1 if 'pthread_condattr_setclock()' is available */
-#cmakedefine H5_HAVE_PTHREAD_CONDATTR_SETCLOCK @H5_HAVE_PTHREAD_CONDATTR_SETCLOCK@
-
-/* Define to 1 if PTHREAD_MUTEX_ADAPTIVE_NP is available */
-#cmakedefine H5_HAVE_PTHREAD_MUTEX_ADAPTIVE_NP @H5_HAVE_PTHREAD_MUTEX_ADAPTIVE_NP@
+/* Define to 1 if the compiler supports the pthread_barrier_*() routines */
+#cmakedefine H5_HAVE_PTHREAD_BARRIER @H5_HAVE_PTHREAD_BARRIER@
 
 /* Define to 1 if you have the <pwd.h> header file. */
 #cmakedefine H5_HAVE_PWD_H @H5_HAVE_PWD_H@
@@ -334,6 +328,9 @@
 /* Define to 1 if you have the <szlib.h> header file. */
 #cmakedefine H5_HAVE_SZLIB_H @H5_HAVE_SZLIB_H@
 
+/* Define to 1 if the compiler supports the __builtin_expect() extension */
+#cmakedefine H5_HAVE_BUILTIN_EXPECT @H5_HAVE_BUILTIN_EXPECT@
+
 #if defined(_WIN32) && !defined(H5_BUILT_AS_DYNAMIC_LIB)
 /* Not supported on WIN32 platforms with static linking */
 /* #undef H5_HAVE_THREADSAFE */

config/cmake/HDF5DeveloperBuild.cmake

@@ -174,6 +174,12 @@ if (HDF5_ENABLE_DEBUG_H5FS_ASSERT)
   list (APPEND HDF5_DEBUG_APIS H5FS_DEBUG_ASSERT)
 endif ()
 
+option (HDF5_ENABLE_DEBUG_H5TS "Enable debugging of H5TS module" OFF)
+mark_as_advanced (HDF5_ENABLE_DEBUG_H5TS)
+if (HDF5_ENABLE_DEBUG_H5TS)
+  list (APPEND HDF5_DEBUG_APIS H5TS_DEBUG)
+endif ()
+
 # If HDF5 free list debugging wasn't specifically enabled, disable
 # free lists entirely for developer build modes, as they can
 # make certain types of issues (like references to stale pointers)

config/cmake/HDFTests.c

@@ -15,6 +15,21 @@
 #define SIMPLE_TEST(x) int main(void){ x; return 0; }
 
 
+#ifdef HAVE_BUILTIN_EXPECT
+
+int
+main ()
+{
+    void *ptr = (void*) 0;
+
+    if (__builtin_expect (ptr != (void*) 0, 1))
+        return 0;
+
+    return 0;
+}
+
+#endif /* HAVE_BUILTIN_EXPECT */
+
 #ifdef HAVE_ATTRIBUTE
 
 int
@@ -37,6 +52,22 @@ SIMPLE_TEST(timezone = 0);
 
 #endif /* HAVE_TIMEZONE */
 
+#ifdef PTHREAD_BARRIER
+#include <pthread.h>
+
+int main(void)
+{
+    pthread_barrier_t barr;
+    int ret;
+
+    ret = pthread_barrier_init(&barr, NULL, 1);
+    if (ret == 0)
+        return 0;
+    return 1;
+}
+
+#endif /* PTHREAD_BARRIER */
+
 #ifdef SYSTEM_SCOPE_THREADS
 #include <stdlib.h>
 #include <pthread.h>

config/gnu-warnings/error-general

@@ -38,8 +38,6 @@
 # Here is a list of tests and examples that have issues with the stricter warnings as error
 #
 # NOTE: Test files are not compatible with these warnings as errors
-#     thread_id.c,
-#       -Werror=unused-function
 #     dsets.c
 #       -Werror=unused-parameter
 #     external.c,perform/sio_engine.c

configure.ac

@@ -1830,8 +1830,13 @@ AM_CONDITIONAL([BUILD_SHARED_SZIP_CONDITIONAL], [test "X$USE_FILTER_SZIP" = "Xye
 AC_CACHE_SAVE
 
 ## ----------------------------------------------------------------------
-## Enable thread-safe version of library.  It requires Pthreads support
-## on POSIX systems.
+## Enable thread-safe version of library (requires Pthreads on POSIX
+## systems). We usually pick up the system Pthreads library, so --with-pthread
+## is only necessary if you are using a custom Pthreads library or if
+## your OS hides its implementation in an unusual location.
+##
+## On Windows, we use Win32 threads and no special configuration should be
+## required to use them.
 ##
 AC_SUBST([THREADSAFE])
 
@@ -1969,6 +1974,15 @@ if test "X$THREADSAFE" = "Xyes"; then
         ;;
     esac
 
+    ## If using Pthreads, check for barrier routines
+    if test "x$HAVE_PTHREAD" = "xyes"; then
+        AC_CHECK_DECL([pthread_barrier_init],
+                      [AC_DEFINE([HAVE_PTHREAD_BARRIER], [1],
+                          [Define if has pthread_barrier_*() routines])],
+                      [],
+                      [[#include <pthread.h>]])
+    fi
+
     ## ----------------------------------------------------------------------
     ## Check if pthread_attr_setscope(&attribute, PTHREAD_SCOPE_SYSTEM)
     ## is supported on this system
@@ -2126,6 +2140,14 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]],[[int __attribute__((unused)) x]])],
                  AC_MSG_RESULT([yes])],
                [AC_MSG_RESULT([no])])
 
+AC_MSG_CHECKING([if compiler supports the __builtin_expect() extension])
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]],[[void *ptr = (void*) 0;
+               if (__builtin_expect (ptr != (void*) 0, 1)) return 0;]])],
+  [AC_DEFINE([HAVE_BUILTIN_EXPECT], [1],
+          [Define if supports __builtin_expect() extension])
+    AC_MSG_RESULT([yes])],
+  [AC_MSG_RESULT([no])])
+
 ## ----------------------------------------------------------------------
 ## Remove old ways of determining debug/production build.
 ## These were used in 1.8.x and earlier. We should probably keep these checks
@@ -2559,8 +2581,8 @@ AC_SUBST([INTERNAL_DEBUG_OUTPUT])
 ## too specialized or have huge performance hits. These
 ## are not listed in the "all" packages list.
 ##
-## all_packages="AC,B2,CX,D,F,FA,FL,FS,MM,O,T,Z"
-all_packages="AC,B2,CX,D,F,MM,O,T,Z"
+## all_packages="AC,B2,CX,D,F,FA,FL,FS,MM,O,T,TS,Z"
+all_packages="AC,B2,CX,D,F,MM,O,T,TS,Z"
 
 case "X-$INTERNAL_DEBUG_OUTPUT" in
   X-yes|X-all)
@@ -3190,26 +3212,8 @@ if test "X$SUBFILING_VFD" = "Xyes"; then
     fi
 
     # Checks for libraries.
-    AC_SEARCH_LIBS([shm_open], [rt])
     AC_CHECK_LIB([pthread], [pthread_self],[], [echo "Error: Required library pthread not found." && exit 1])
 
-    # Perform various checks for Mercury util code
-    AC_CHECK_DECL([PTHREAD_MUTEX_ADAPTIVE_NP],
-                  [AC_DEFINE([HAVE_PTHREAD_MUTEX_ADAPTIVE_NP], [1],
-                      [Define if has PTHREAD_MUTEX_ADAPTIVE_NP])],
-                  [],
-                  [[#include <pthread.h>]])
-    AC_CHECK_DECL([pthread_condattr_setclock],
-                  [AC_DEFINE([HAVE_PTHREAD_CONDATTR_SETCLOCK], [1],
-                      [Define if has pthread_condattr_setclock()])],
-                  [],
-                  [[#include <pthread.h>]])
-    AC_CHECK_DECL([CLOCK_MONOTONIC_COARSE],
-                  [AC_DEFINE([HAVE_CLOCK_MONOTONIC_COARSE], [1],
-                      [Define if has CLOCK_MONOTONIC_COARSE])],
-                  [],
-                  [[#include <time.h>]])
-
 else
     AC_MSG_RESULT([no])
 fi

release_docs/RELEASE.txt

@@ -1118,13 +1118,6 @@ Bug Fixes since HDF5-1.14.0 release
       of HDF5. The CMake configuration code already avoids installing the
       executable on the system.
 
-    - Fixed a configuration issue that prevented building of the Subfiling VFD on macOS
-
-      Checks were added to the CMake and Autotools code to verify that CLOCK_MONOTONIC_COARSE,
-      PTHREAD_MUTEX_ADAPTIVE_NP and pthread_condattr_setclock() are available before attempting
-      to use them in Subfiling VFD-related utility code. Without these checks, attempting
-      to build the Subfiling VFD on macOS would fail.
-
     - Fixes the ordering of INCLUDES when building with CMake
 
       Include directories in the source or build tree should come before other

src/CMakeLists.txt

@@ -640,6 +640,17 @@ IDE_GENERATED_PROPERTIES ("H5T" "${H5T_HDRS}" "${H5T_SOURCES}" )
 
 set (H5TS_SOURCES
     ${HDF5_SRC_DIR}/H5TS.c
+    ${HDF5_SRC_DIR}/H5TSbarrier.c
+    ${HDF5_SRC_DIR}/H5TScond.c
+    ${HDF5_SRC_DIR}/H5TSexlock.c
+    ${HDF5_SRC_DIR}/H5TSint.c
+    ${HDF5_SRC_DIR}/H5TSkey.c
+    ${HDF5_SRC_DIR}/H5TSmutex.c
+    ${HDF5_SRC_DIR}/H5TSpthread.c
+    ${HDF5_SRC_DIR}/H5TSrwlock.c
+    ${HDF5_SRC_DIR}/H5TStest.c
+    ${HDF5_SRC_DIR}/H5TSthread.c
+    ${HDF5_SRC_DIR}/H5TSwin.c
 )
 set (H5TS_HDRS
     ${HDF5_SRC_DIR}/H5TSdevelop.h
@@ -757,6 +768,7 @@ set (H5_MODULE_HEADERS
     ${HDF5_SRC_DIR}/H5SLmodule.h
     ${HDF5_SRC_DIR}/H5SMmodule.h
     ${HDF5_SRC_DIR}/H5Tmodule.h
+    ${HDF5_SRC_DIR}/H5TSmodule.h
     ${HDF5_SRC_DIR}/H5VLmodule.h
     ${HDF5_SRC_DIR}/H5Zmodule.h
 )
@@ -958,6 +970,7 @@ set (H5_PRIVATE_HEADERS
     ${HDF5_SRC_DIR}/H5Tpkg.h
     ${HDF5_SRC_DIR}/H5Tprivate.h
 
+    ${HDF5_SRC_DIR}/H5TSpkg.h
     ${HDF5_SRC_DIR}/H5TSprivate.h
 
     ${HDF5_SRC_DIR}/H5UCprivate.h

src/H5.c

@@ -73,14 +73,9 @@ static int H5__mpi_delete_cb(MPI_Comm comm, int keyval, void *attr_val, int *fla
 static const unsigned VERS_RELEASE_EXCEPTIONS[]    = {0};
 static const unsigned VERS_RELEASE_EXCEPTIONS_SIZE = 1;
 
-/* statically initialize block for pthread_once call used in initializing */
-/* the first global mutex                                                 */
-#ifdef H5_HAVE_THREADSAFE
-H5_api_t H5_g;
-#else
+/* Library init / term status (global) */
 bool H5_libinit_g = false; /* Library hasn't been initialized */
 bool H5_libterm_g = false; /* Library isn't being shutdown */
-#endif
 
 char        H5_lib_vers_info_g[] = H5_VERS_INFO;
 static bool H5_dont_atexit_g     = false;
@@ -213,12 +208,13 @@ H5_init_library(void)
      */
     if (!H5_dont_atexit_g) {
 
-#if defined(H5_HAVE_THREADSAFE) && defined(H5_HAVE_WIN_THREADS)
-        /* Clean up Win32 thread resources. Pthreads automatically cleans up.
-         * This must be entered before the library cleanup code so it's
+#if defined(H5_HAVE_THREADSAFE)
+        /* Clean up thread resources.
+         *
+         * This must be pushed before the library cleanup code so it's
          * executed in LIFO order (i.e., last).
          */
-        (void)atexit(H5TS_win32_process_exit);
+        (void)atexit(H5TS_term_package);
 #endif /* H5_HAVE_THREADSAFE && H5_HAVE_WIN_THREADS */
 
         /* Normal library termination code */
@@ -303,13 +299,12 @@ H5_term_library(void)
     H5E_auto2_t func;
 
 #ifdef H5_HAVE_THREADSAFE
-    /* explicit locking of the API */
-    H5_FIRST_THREAD_INIT
+    /* explicitly lock the API */
     H5_API_LOCK
 #endif
 
     /* Don't do anything if the library is already closed */
-    if (!(H5_INIT_GLOBAL))
+    if (!H5_INIT_GLOBAL)
         goto done;
 
     /* Indicate that the library is being shut down */
@@ -1125,7 +1120,7 @@ H5allocate_memory(size_t size, bool clear)
 {
     void *ret_value = NULL;
 
-    FUNC_ENTER_API_NOINIT
+    FUNC_ENTER_API_REENTER
     H5TRACE2("*x", "zb", size, clear);
 
     if (0 == size)
@@ -1136,7 +1131,7 @@ H5allocate_memory(size_t size, bool clear)
     else
         ret_value = H5MM_malloc(size);
 
-    FUNC_LEAVE_API_NOINIT(ret_value)
+    FUNC_LEAVE_API_REENTER(ret_value)
 } /* end H5allocate_memory() */
 
 /*-------------------------------------------------------------------------
@@ -1168,12 +1163,12 @@ H5resize_memory(void *mem, size_t size)
 {
     void *ret_value = NULL;
 
-    FUNC_ENTER_API_NOINIT
+    FUNC_ENTER_API_REENTER
     H5TRACE2("*x", "*xz", mem, size);
 
     ret_value = H5MM_realloc(mem, size);
 
-    FUNC_LEAVE_API_NOINIT(ret_value)
+    FUNC_LEAVE_API_REENTER(ret_value)
 } /* end H5resize_memory() */
 
 /*-------------------------------------------------------------------------
@@ -1191,13 +1186,13 @@ H5resize_memory(void *mem, size_t size)
 herr_t
 H5free_memory(void *mem)
 {
-    FUNC_ENTER_API_NOINIT
+    FUNC_ENTER_API_REENTER
     H5TRACE1("e", "*x", mem);
 
     /* At this time, it is impossible for this to fail. */
     H5MM_xfree(mem);
 
-    FUNC_LEAVE_API_NOINIT(SUCCEED)
+    FUNC_LEAVE_API_REENTER(SUCCEED)
 } /* end H5free_memory() */
 
 /*-------------------------------------------------------------------------