Moved thread cancel state management out of global API locking code.

Also refactored the exclusive lock 'lock' operation a bit to make it more clear.

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

src/H5.c

@@ -298,10 +298,9 @@ H5_term_library(void)
     int         nprinted;
     H5E_auto2_t func;
 
-#ifdef H5_HAVE_THREADSAFE
-    /* explicitly lock the API */
+    /* Acquire the API lock */
+    H5CANCEL_DECL
     H5_API_LOCK
-#endif
 
     /* Don't do anything if the library is already closed */
     if (!H5_INIT_GLOBAL)
@@ -501,9 +500,8 @@ H5_term_library(void)
     /* Don't pop the API context (i.e. H5CX_pop), since it's been shut down already */
 
 done:
-#ifdef H5_HAVE_THREADSAFE
+    /* Release API lock */
     H5_API_UNLOCK
-#endif /* H5_HAVE_THREADSAFE */
 
     return;
 } /* end H5_term_library() */
@@ -735,7 +733,7 @@ H5__debug_mask(const char *s)
                         H5_debug_g.pkg[i].stream = clear ? NULL : stream;
                         break;
                     } /* end if */
-                }     /* end for */
+                } /* end for */
                 if (i >= (size_t)H5_NPKGS)
                     fprintf(stderr, "HDF5_DEBUG: ignored %s\n", pkg_name);
             } /* end if-else */
@@ -763,7 +761,7 @@ H5__debug_mask(const char *s)
         else {
             s++;
         } /* end if-else */
-    }     /* end while */
+    } /* end while */
 } /* end H5__debug_mask() */
 
 #ifdef H5_HAVE_PARALLEL

src/H5FDtest.c

@@ -81,6 +81,7 @@ H5FDopen_test(const char *name, unsigned flags, hid_t fapl_id, haddr_t maxaddr)
     H5FD_t *ret_value;
 
     FUNC_ENTER_API(NULL)
+    H5TRACE4("*#", "*sIuia", name, flags, fapl_id, maxaddr);
 
     /* Call developer routine */
     ret_value = H5FDopen(name, flags, fapl_id, maxaddr);
@@ -108,6 +109,7 @@ H5FDclose_test(H5FD_t *file)
     herr_t ret_value;
 
     FUNC_ENTER_API(FAIL)
+    H5TRACE1("e", "*#", file);
 
     /* Call developer routine */
     ret_value = H5FDclose(file);
@@ -135,6 +137,7 @@ H5FDalloc_test(H5FD_t *file, H5FD_mem_t type, hid_t dxpl_id, hsize_t size)
     haddr_t ret_value;
 
     FUNC_ENTER_API(HADDR_UNDEF)
+    H5TRACE4("a", "*#Mtih", file, type, dxpl_id, size);
 
     /* Call developer routine */
     ret_value = H5FDalloc(file, type, dxpl_id, size);
@@ -162,6 +165,7 @@ H5FDget_eoa_test(H5FD_t *file, H5FD_mem_t type)
     haddr_t ret_value;
 
     FUNC_ENTER_API(HADDR_UNDEF)
+    H5TRACE2("a", "*#Mt", file, type);
 
     /* Call developer routine */
     ret_value = H5FDget_eoa(file, type);
@@ -189,6 +193,7 @@ H5FDset_eoa_test(H5FD_t *file, H5FD_mem_t type, haddr_t eoa)
     herr_t ret_value;
 
     FUNC_ENTER_API(FAIL)
+    H5TRACE3("e", "*#Mta", file, type, eoa);
 
     /* Call developer routine */
     ret_value = H5FDset_eoa(file, type, eoa);
@@ -216,6 +221,7 @@ H5FDget_eof_test(H5FD_t *file, H5FD_mem_t type)
     haddr_t ret_value;
 
     FUNC_ENTER_API(HADDR_UNDEF)
+    H5TRACE2("a", "*#Mt", file, type);
 
     /* Call developer routine */
     ret_value = H5FDget_eof(file, type);
@@ -243,6 +249,7 @@ H5FDread_test(H5FD_t *file, H5FD_mem_t type, hid_t dxpl_id, haddr_t addr, size_t
     herr_t ret_value;
 
     FUNC_ENTER_API(FAIL)
+    H5TRACE6("e", "*#Mtiaz*x", file, type, dxpl_id, addr, size, buf);
 
     /* Call developer routine */
     ret_value = H5FDread(file, type, dxpl_id, addr, size, buf);
@@ -270,6 +277,7 @@ H5FDwrite_test(H5FD_t *file, H5FD_mem_t type, hid_t dxpl_id, haddr_t addr, size_
     herr_t ret_value;
 
     FUNC_ENTER_API(FAIL)
+    H5TRACE6("e", "*#Mtiaz*x", file, type, dxpl_id, addr, size, buf);
 
     /* Call developer routine */
     ret_value = H5FDwrite(file, type, dxpl_id, addr, size, buf);
@@ -298,6 +306,7 @@ H5FDread_vector_test(H5FD_t *file, hid_t dxpl_id, uint32_t count, H5FD_mem_t typ
     herr_t ret_value;
 
     FUNC_ENTER_API(FAIL)
+    H5TRACE7("e", "*#iIu*Mt*a*z**x", file, dxpl_id, count, types, addrs, sizes, bufs);
 
     /* Call developer routine */
     ret_value = H5FDread_vector(file, dxpl_id, count, types, addrs, sizes, bufs);
@@ -326,6 +335,7 @@ H5FDwrite_vector_test(H5FD_t *file, hid_t dxpl_id, uint32_t count, H5FD_mem_t ty
     herr_t ret_value;
 
     FUNC_ENTER_API(FAIL)
+    H5TRACE7("e", "*#iIu*Mt*a*z**x", file, dxpl_id, count, types, addrs, sizes, bufs);
 
     /* Call developer routine */
     ret_value = H5FDwrite_vector(file, dxpl_id, count, types, addrs, sizes, bufs);
@@ -354,6 +364,8 @@ H5FDread_selection_test(H5FD_t *file, H5FD_mem_t type, hid_t dxpl_id, uint32_t c
     herr_t ret_value;
 
     FUNC_ENTER_API(FAIL)
+    H5TRACE9("e", "*#MtiIu*i*i*a*z**x", file, type, dxpl_id, count, mem_spaces, file_spaces, offsets,
+             element_sizes, bufs);
 
     /* Call developer routine */
     ret_value =
@@ -383,6 +395,8 @@ H5FDwrite_selection_test(H5FD_t *file, H5FD_mem_t type, hid_t dxpl_id, uint32_t
     herr_t ret_value;
 
     FUNC_ENTER_API(FAIL)
+    H5TRACE9("e", "*#MtiIu*i*i*a*z**x", file, type, dxpl_id, count, mem_spaces, file_spaces, offsets,
+             element_sizes, bufs);
 
     /* Call developer routine */
     ret_value = H5FDwrite_selection(file, type, dxpl_id, count, mem_spaces, file_spaces, offsets,
@@ -411,6 +425,7 @@ H5FDtruncate_test(H5FD_t *file, hid_t dxpl_id, hbool_t closing)
     herr_t ret_value;
 
     FUNC_ENTER_API(FAIL)
+    H5TRACE3("e", "*#ib", file, dxpl_id, closing);
 
     /* Call developer routine */
     ret_value = H5FDtruncate(file, dxpl_id, closing);
@@ -438,6 +453,7 @@ H5FDctl_test(H5FD_t *file, uint64_t op_code, uint64_t flags, const void *input,
     herr_t ret_value;
 
     FUNC_ENTER_API(FAIL)
+    H5TRACE5("e", "*#ULUL*x**x", file, op_code, flags, input, output);
 
     /* Call developer routine */
     ret_value = H5FDctl(file, op_code, flags, input, output);

src/H5TSexlock.c

@@ -74,7 +74,7 @@ static const H5TS_ex_lock_t H5TS_ex_lock_def = H5TS_EX_LOCK_INIT;
  *--------------------------------------------------------------------------
  */
 herr_t
-H5TS__ex_lock_init(H5TS_ex_lock_t *lock, bool disable_cancel)
+H5TS__ex_lock_init(H5TS_ex_lock_t *lock)
 {
     herr_t ret_value = SUCCEED;
 
@@ -86,13 +86,6 @@ H5TS__ex_lock_init(H5TS_ex_lock_t *lock, bool disable_cancel)
     /* Initialize the lock */
     memcpy(lock, &H5TS_ex_lock_def, sizeof(H5TS_ex_lock_def));
 
-#ifdef H5_HAVE_PTHREAD_H
-    /* Set non-default fields */
-    lock->disable_cancel = disable_cancel;
-#else
-    (void)disable_cancel;
-#endif
-
 done:
     FUNC_LEAVE_NOAPI_NAMECHECK_ONLY(ret_value)
 } /* end H5TS__ex_lock_init() */
@@ -120,29 +113,29 @@ H5TS__ex_lock(H5TS_ex_lock_t *lock)
         HGOTO_DONE(FAIL);
     have_mutex = true;
 
-    /* Check if this thread already owns the lock */
-    if (lock->lock_count && H5TS_thread_equal(my_thread_id, lock->owner_thread))
-        /* Already owned by self - increment count */
-        lock->lock_count++;
-    else {
-        /* Wait until the mutex is released by current owner thread */
-        while (lock->lock_count)
-            if (H5_UNLIKELY(H5TS_cond_wait(&lock->cond_var, &lock->mutex) < 0))
-                HGOTO_DONE(FAIL);
+    /* Check if the lock is already owned */
+    if (lock->lock_count) {
+        /* Does this thread already own the lock? */
+        if (H5TS_thread_equal(my_thread_id, lock->owner_thread)) {
+            /* Already owned by self - increment count */
+            lock->lock_count++;
 
-        /* After we've received the signal, take ownership of the lock */
-        lock->owner_thread = my_thread_id;
-        lock->lock_count   = 1;
-
-#ifdef H5_HAVE_PTHREAD_H
-        /* Disable cancellation, if requested for this lock */
-        if (lock->disable_cancel)
-            /* Set cancellation state to 'disable', and remember previous state */
-            if (H5_UNLIKELY(pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &lock->previous_state)))
-                HGOTO_DONE(FAIL);
-#endif
+            /* Leave now, we already own this lock */
+            HGOTO_DONE(SUCCEED);
+        }
+        else {
+            /* Wait until the mutex is released by current owner thread */
+            do {
+                if (H5_UNLIKELY(H5TS_cond_wait(&lock->cond_var, &lock->mutex) < 0))
+                    HGOTO_DONE(FAIL);
+            } while (lock->lock_count);
+        }
     }
 
+    /* Take ownership of the lock */
+    lock->owner_thread = my_thread_id;
+    lock->lock_count   = 1;
+
 done:
     if (H5_LIKELY(have_mutex))
         if (H5_UNLIKELY(H5TS_mutex_unlock(&lock->mutex) < 0))
@@ -268,15 +261,8 @@ H5TS__ex_unlock(H5TS_ex_lock_t *lock)
     /* Decrement the lock count for this thread */
     lock->lock_count--;
 
-    if (lock->lock_count == 0) {
-#ifdef H5_HAVE_PTHREAD_H
-        /* Restore previous cancellation state, if requested for this lock */
-        if (lock->disable_cancel)
-            if (H5_UNLIKELY(pthread_setcancelstate(lock->previous_state, NULL)))
-                HGOTO_DONE(FAIL);
-#endif
-
-        /* If the lock count drops to zero, signal the condition variable, to
+    if (0 == lock->lock_count) {
+        /* If the lock count drops to zero, signal the condition variable to
          * wake any thread waiting on the lock.
          */
         if (H5_UNLIKELY(H5TS_cond_signal(&lock->cond_var) < 0))

src/H5TSpkg.h

@@ -74,25 +74,13 @@
 #endif
 
 /* Excl lock initialization macro */
-#ifdef H5_HAVE_PTHREAD_H
-#define H5TS_EX_LOCK_INIT                                                                                    \
-    {                                                                                                        \
-        H5TS_MUTEX_INITIALIZER,    /* mutex */                                                               \
-            H5TS_COND_INITIALIZER, /* cond_var */                                                            \
-            0,                     /* owner_thread */                                                        \
-            0,                     /* lock_count */                                                          \
-            false,                 /* disable_cancel */                                                      \
-            0                      /* previous_state */                                                      \
-    }
-#else
 #define H5TS_EX_LOCK_INIT                                                                                    \
     {                                                                                                        \
         H5TS_MUTEX_INITIALIZER,    /* mutex */                                                               \
             H5TS_COND_INITIALIZER, /* cond_var */                                                            \
             0,                     /* owner_thread */                                                        \
             0                      /* lock_count */                                                          \
     }
-#endif
 
 /****************************/
 /* Package Private Typedefs */
@@ -104,13 +92,6 @@ typedef struct H5TS_ex_lock_t {
     H5TS_cond_t   cond_var;
     H5TS_thread_t owner_thread;
     unsigned      lock_count;
-
-/* Thread cancellability only supported with pthreads */
-#ifdef H5_HAVE_PTHREAD_H
-    /* Cancellation control */
-    bool disable_cancel;
-    int  previous_state;
-#endif /* H5_HAVE_PTHREAD_H */
 } H5TS_ex_lock_t;
 
 /* Thread Barrier */
@@ -318,7 +299,7 @@ H5_DLL herr_t H5TS__rw_unlock(H5TS_rw_lock_t *rw_lock);
 H5_DLL herr_t H5TS__rw_lock_destroy(H5TS_rw_lock_t *rw_lock);
 
 /* Recursive exclusive lock related function declarations */
-H5_DLL herr_t H5TS__ex_lock_init(H5TS_ex_lock_t *lock, bool disable_cancel);
+H5_DLL herr_t H5TS__ex_lock_init(H5TS_ex_lock_t *lock);
 H5_DLL herr_t H5TS__ex_lock(H5TS_ex_lock_t *lock);
 H5_DLL herr_t H5TS__ex_acquire(H5TS_ex_lock_t *lock, unsigned lock_count, bool *acquired);
 H5_DLL herr_t H5TS__ex_release(H5TS_ex_lock_t *lock, unsigned int *lock_count);

src/H5TSprivate.h

@@ -52,20 +52,22 @@
 #define H5TS_ONCE_INITIALIZER INIT_ONCE_STATIC_INIT
 
 /* Thread macros */
-#define H5TS_thread_self()        GetCurrentThread()
-#define H5TS_thread_equal(t1, t2) (GetThreadId(t1) == GetThreadId(t2))
-#define H5TS_THREAD_RETURN_TYPE   H5TS_thread_ret_t WINAPI
+#define H5TS_thread_self()         GetCurrentThread()
+#define H5TS_thread_equal(t1, t2)  (GetThreadId(t1) == GetThreadId(t2))
+#define H5TS_THREAD_RETURN_TYPE    H5TS_thread_ret_t WINAPI
+#define H5TS_THREAD_CANCEL_DISABLE 0
 #else
 /* Static initialization values */
-#define H5TS_KEY_INITIALIZER      (pthread_key_t)0
-#define H5TS_MUTEX_INITIALIZER    PTHREAD_MUTEX_INITIALIZER
-#define H5TS_COND_INITIALIZER     PTHREAD_COND_INITIALIZER
-#define H5TS_ONCE_INITIALIZER     PTHREAD_ONCE_INIT
+#define H5TS_KEY_INITIALIZER       (pthread_key_t)0
+#define H5TS_MUTEX_INITIALIZER     PTHREAD_MUTEX_INITIALIZER
+#define H5TS_COND_INITIALIZER      PTHREAD_COND_INITIALIZER
+#define H5TS_ONCE_INITIALIZER      PTHREAD_ONCE_INIT
 
 /* Thread macros */
-#define H5TS_thread_self()        pthread_self()
-#define H5TS_thread_equal(t1, t2) pthread_equal((t1), (t2))
-#define H5TS_THREAD_RETURN_TYPE   H5TS_thread_ret_t
+#define H5TS_thread_self()         pthread_self()
+#define H5TS_thread_equal(t1, t2)  pthread_equal((t1), (t2))
+#define H5TS_THREAD_RETURN_TYPE    H5TS_thread_ret_t
+#define H5TS_THREAD_CANCEL_DISABLE PTHREAD_CANCEL_DISABLE
 #endif
 
 /****************************/
@@ -147,6 +149,7 @@ H5_DLL herr_t H5TS_key_delete(H5TS_key_t key);
 H5_DLL herr_t H5TS_thread_create(H5TS_thread_t *thread, H5TS_thread_start_func_t func, void *udata);
 H5_DLL herr_t H5TS_thread_join(H5TS_thread_t thread, H5TS_thread_ret_t *ret_val);
 H5_DLL herr_t H5TS_thread_detach(H5TS_thread_t thread);
+H5_DLL herr_t H5TS_thread_setcancelstate(int state, int *oldstate);
 
 /* Thread pools */
 H5_DLL herr_t H5TS_pool_create(H5TS_pool_t **pool, unsigned num_threads);

src/H5TSpthread.c

@@ -77,9 +77,6 @@ H5TS__pthread_first_thread_init(void)
 {
     FUNC_ENTER_NOAPI_NAMECHECK_ONLY
 
-    /* Initialize global API lock, to disable cancels */
-    H5TS__ex_lock_init(&H5TS_api_info_p.api_lock, true);
-
     /* Set up thread-local thread-info struct */
     H5TS__tinfo_init();