[svn-r24961] Updates to Win32 thread-local storage cleanup when the thread-safe library is built on Windows. Previously, thread-local storage was not cleaned up, causing resource leaks.

Fixes HDFFV-8518, HDFFV-8699

As a part of these changes, the thread-safe + static library options are declared unsupported since the solution relies on DllMain. A solution for the static library is probably doable, but requires much more complicated surgery and has been deferred to HDF5 1.8.14.

Tested on:
    64-bit Windows 7 using VS 2012 (changes only affect Windows)

6 files changed, 254 insertions, 77 deletions

diff --git a/src/H5.c b/src/H5.c
index 1fb5baa..9ece760 100644
--- a/src/H5.c
+++ b/src/H5.c
@@ -151,13 +151,23 @@ H5_init_library(void)
 #endif
 
     /*
-     * Install atexit() library cleanup routine unless the H5dont_atexit()
+     * Install atexit() library cleanup routines unless the H5dont_atexit()
      * has been called.  Once we add something to the atexit() list it stays
      * there permanently, so we set H5_dont_atexit_g after we add it to prevent
      * adding it again later if the library is cosed and reopened.
      */
     if (!H5_dont_atexit_g) {
-	(void)HDatexit(H5_term_library);
+
+#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
+         * executed in LIFO order (i.e., last).
+         */
+	    (void)HDatexit(H5TS_win32_process_exit);
+#endif /* H5_HAVE_THREADSAFE && H5_HAVE_WIN_THREADS */
+
+        /* Normal library termination code */
+        (void)HDatexit(H5_term_library);
 	H5_dont_atexit_g = TRUE;
     } /* end if */
 
@@ -330,7 +340,7 @@ H5_term_library(void)
 done:
 #ifdef H5_HAVE_THREADSAFE
     H5_API_UNLOCK
-#endif
+#endif /* H5_HAVE_THREADSAFE */
     return;
 } /* end H5_term_library() */
 
@@ -831,3 +841,60 @@ H5free_memory(void *mem)
     FUNC_LEAVE_API(SUCCEED)
 } /* end H5free_memory() */
 
+
+#ifdef H5_HAVE_WIN32_API
+/*-------------------------------------------------------------------------
+ * Function:    DllMain
+ *
+ * Purpose:     Handles various conditions in the library on Windows.
+ *
+ *    NOTE:     The main purpose of this is for handling Win32 thread cleanup
+ *              on thread/process detach.
+ *
+ * Return:      TRUE on success, FALSE on failure
+ *
+ *-------------------------------------------------------------------------
+ */
+BOOL WINAPI
+DllMain(_In_ HINSTANCE hinstDLL, _In_ DWORD fdwReason, _In_ LPVOID lpvReserved)
+{
+    /* Don't add our function enter/leave macros since this function will be
+     * called before the library is initialized.
+     *
+     * NOTE: Do NOT call any CRT functions in DllMain!
+     * This includes any functions that are called by from here!
+     */
+
+    BOOL fOkay = TRUE;
+
+    switch(fdwReason)
+    {
+    case DLL_PROCESS_ATTACH:
+        break;
+
+    case DLL_PROCESS_DETACH:
+        break;
+
+    case DLL_THREAD_ATTACH:
+#ifdef H5_HAVE_WIN_THREADS
+        if(H5TS_win32_thread_enter() < 0)
+            fOkay = FALSE;
+#endif /* H5_HAVE_WIN_THREADS */
+        break;
+
+    case DLL_THREAD_DETACH:
+#ifdef H5_HAVE_WIN_THREADS
+        if(H5TS_win32_thread_exit() < 0)
+            fOkay = FALSE;
+#endif /* H5_HAVE_WIN_THREADS */
+        break;
+
+    default:
+        /* Shouldn't get here */
+        fOkay = FALSE;
+        break;
+    }
+
+    return fOkay;
+}
+#endif /* H5_HAVE_WIN32_API */
diff --git a/src/H5CS.c b/src/H5CS.c
index 4ccd0f8..29182ce 100644
--- a/src/H5CS.c
+++ b/src/H5CS.c
@@ -83,12 +83,16 @@ H5CS_get_stack(void)
 
     fstack = H5TS_get_thread_local_value(H5TS_funcstk_key_g);
     if (!fstack) {
-        /* no associated value with current thread - create one */
-        fstack = (H5CS_t *)HDmalloc(sizeof(H5CS_t));  /* Don't use H5MM_malloc() here, it causes infinite recursion */
+        /* No associated value with current thread - create one */
+#ifdef H5_HAVE_WIN_THREADS
+        fstack = (H5CS_t *)LocalAlloc(LPTR, sizeof(H5CS_t)); /* Win32 has to use LocalAlloc to match the LocalFree in DllMain */
+#else
+        fstack = (H5CS_t *)HDmalloc(sizeof(H5CS_t)); /* Don't use H5MM_malloc() here, it causes infinite recursion */
+#endif /* H5_HAVE_WIN_THREADS */
         HDassert(fstack);
 
         /* Set the thread-specific info */
-	fstack->nused=0;
+        fstack->nused=0;
 
         /* (It's not necessary to release this in this API, it is
          *      released by the "key destructor" set up in the H5TS
diff --git a/src/H5E.c b/src/H5E.c
index 3fc158c..73f1d51 100644
--- a/src/H5E.c
+++ b/src/H5E.c
@@ -363,8 +363,12 @@ H5E_get_stack(void)
     estack = (H5E_t *)H5TS_get_thread_local_value(H5TS_errstk_key_g);
 
     if(!estack) {
-        /* no associated value with current thread - create one */
-        estack = (H5E_t *)H5FL_MALLOC(H5E_t);
+        /* No associated value with current thread - create one */
+#ifdef H5_HAVE_WIN_THREADS
+        estack = (H5E_t *)LocalAlloc(LPTR, sizeof(H5E_t)); /* Win32 has to use LocalAlloc to match the LocalFree in DllMain */
+#else
+        estack = (H5E_t *)H5FL_MALLOC(sizeof(H5E_t));
+#endif /* H5_HAVE_WIN_THREADS */
         HDassert(estack);
 
         /* Set the thread-specific info */
diff --git a/src/H5TS.c b/src/H5TS.c
index 6f661ad..b69bd4d 100644
--- a/src/H5TS.c
+++ b/src/H5TS.c
@@ -56,8 +56,6 @@ H5TS_key_t H5TS_cancel_key_g;
  * PROGRAMMER: Quincey Koziol
  *             February 7, 2003
  *
- * MODIFICATIONS:
- *
  *--------------------------------------------------------------------------
  */
 static void
@@ -68,39 +66,9 @@ H5TS_key_destructor(void *key_val)
         HDfree(key_val);
 }
 
-#ifdef H5_HAVE_WIN_THREADS
 
-/*--------------------------------------------------------------------------
- * NAME
- *    H5TS_win32_first_thread_init
- *
- * USAGE
- *    H5TS_win32_first_thread_init()
- *
- * RETURNS
- *
- * DESCRIPTION
- *    Special function on windows needed to call the H5TS_first_thread_init
- *    function.
- *
- * PROGRAMMER: Mike McGreevy
- *             September 1, 2010
- *
- * MODIFICATIONS:
- *
- *--------------------------------------------------------------------------
- */
-BOOL CALLBACK 
-H5TS_win32_first_thread_init(PINIT_ONCE InitOnce, PVOID Parameter, PVOID *lpContex)
-{
-    InitializeCriticalSection ( &H5_g.init_lock.CriticalSection );
-    H5TS_errstk_key_g = TlsAlloc();
-    H5TS_funcstk_key_g = TlsAlloc();
-    H5TS_cancel_key_g = TlsAlloc();
+#ifndef H5_HAVE_WIN_THREADS
 
-    return TRUE;
-} /* H5TS_win32_first_thread_init() */
-#else /* H5_HAVE_WIN_THREADS */
 /*--------------------------------------------------------------------------
  * NAME
  *    H5TS_pthread_first_thread_init
@@ -118,8 +86,6 @@ H5TS_win32_first_thread_init(PINIT_ONCE InitOnce, PVOID Parameter, PVOID *lpCont
  * PROGRAMMER: Chee Wai LEE
  *             May 2, 2000
  *
- * MODIFICATIONS:
- *
  *--------------------------------------------------------------------------
  */
 void
@@ -149,6 +115,7 @@ H5TS_pthread_first_thread_init(void)
 }
 #endif /* H5_HAVE_WIN_THREADS */
 
+
 /*--------------------------------------------------------------------------
  * NAME
  *    H5TS_mutex_lock
@@ -167,11 +134,6 @@ H5TS_pthread_first_thread_init(void)
  * PROGRAMMER: Chee Wai LEE
  *             May 2, 2000
  *
- * MODIFICATIONS:
- *
- *    19 May 2000, Bill Wendling
- *    Changed (*foo). form of accessing structure members to the -> form.
- *
  *--------------------------------------------------------------------------
  */
 herr_t
@@ -203,6 +165,7 @@ H5TS_mutex_lock(H5TS_mutex_t *mutex)
 #endif /* H5_HAVE_WIN_THREADS */
 }
 
+
 /*--------------------------------------------------------------------------
  * NAME
  *    H5TS_mutex_unlock
@@ -221,12 +184,6 @@ H5TS_mutex_lock(H5TS_mutex_t *mutex)
  * PROGRAMMER: Chee Wai LEE
  *             May 2, 2000
  *
- * MODIFICATIONS:
- *
- *    19 May 2000, Bill Wendling
- *    Changed (*foo). form of accessing structure members to the -> form.
- *    Also gave the function a return value.
- *
  *--------------------------------------------------------------------------
  */
 herr_t
@@ -258,6 +215,7 @@ H5TS_mutex_unlock(H5TS_mutex_t *mutex)
 #endif /* H5_HAVE_WIN_THREADS */
 } /* H5TS_mutex_unlock */
 
+
 /*--------------------------------------------------------------------------
  * NAME
  *    H5TS_cancel_count_inc
@@ -280,12 +238,6 @@ H5TS_mutex_unlock(H5TS_mutex_t *mutex)
  * PROGRAMMER: Chee Wai LEE
  *            May 2, 2000
  *
- * MODIFICATIONS:
- *
- *    19 May 2000, Bill Wendling
- *    Changed function to return a value. Also changed the malloc() call to
- *    the H5MM_malloc() call and checked the returned pointer.
- *
  *--------------------------------------------------------------------------
  */
 herr_t
@@ -328,6 +280,7 @@ H5TS_cancel_count_inc(void)
 #endif /* H5_HAVE_WIN_THREADS */
 }
 
+
 /*--------------------------------------------------------------------------
  * NAME
  *    H5TS_cancel_count_dec
@@ -348,11 +301,6 @@ H5TS_cancel_count_inc(void)
  * PROGRAMMER: Chee Wai LEE
  *             May 2, 2000
  *
- * MODIFICATIONS:
- *
- *    19 May 2000, Bill Wendling
- *    Changed so that function returns a value. May be of limited usefulness.
- *
  *--------------------------------------------------------------------------
  */
 herr_t
@@ -377,6 +325,154 @@ H5TS_cancel_count_dec(void)
 }
 
 
+#ifdef H5_HAVE_WIN_THREADS
+/*--------------------------------------------------------------------------
+ * NAME
+ *    H5TS_win32_process_enter
+ *
+ * RETURNS
+ *    SUCCEED/FAIL
+ *
+ * DESCRIPTION
+ *    Per-process setup on Windows when using Win32 threads.
+ *
+ *--------------------------------------------------------------------------
+ */
+H5_DLL BOOL CALLBACK 
+H5TS_win32_process_enter(PINIT_ONCE InitOnce, PVOID Parameter, PVOID *lpContex)
+{
+    BOOL ret_value = TRUE;
+
+    /* Initialize the critical section (can't fail) */
+    InitializeCriticalSection(&H5_g.init_lock.CriticalSection);
+
+    /* Set up thread local storage */
+    if(TLS_OUT_OF_INDEXES == (H5TS_errstk_key_g = TlsAlloc()))
+        ret_value = FALSE;
+
+#ifdef H5_HAVE_CODESTACK
+    if(TLS_OUT_OF_INDEXES == (H5TS_funcstk_key_g = TlsAlloc()))
+        ret_value = FALSE;
+#endif /* H5_HAVE_CODESTACK */
+
+    return ret_value;
+} /* H5TS_win32_process_enter() */
+#endif /* H5_HAVE_WIN_THREADS */
+
+
+#ifdef H5_HAVE_WIN_THREADS
+/*--------------------------------------------------------------------------
+ * NAME
+ *    H5TS_win32_thread_enter
+ *
+ * RETURNS
+ *    SUCCEED/FAIL
+ *
+ * DESCRIPTION
+ *    Per-thread setup on Windows when using Win32 threads.
+ *
+ *--------------------------------------------------------------------------
+ */
+herr_t
+H5TS_win32_thread_enter(void)
+{
+    herr_t ret_value = SUCCEED;
+
+    /* Currently a placeholder function.  TLS setup is performed
+     * elsewhere in the library.
+     *
+     * WARNING: Do NOT use C standard library functions here.
+     * CRT functions are not allowed in DllMain, which is where this code
+     * is used.
+     */
+
+    return ret_value;
+} /* H5TS_win32_thread_enter() */
+#endif /* H5_HAVE_WIN_THREADS */
+
+
+#ifdef H5_HAVE_WIN_THREADS
+/*--------------------------------------------------------------------------
+ * NAME
+ *    H5TS_win32_process_exit
+ *
+ * RETURNS
+ *    SUCCEED/FAIL
+ *
+ * DESCRIPTION
+ *    Per-process cleanup on Windows when using Win32 threads.
+ *
+ *--------------------------------------------------------------------------
+ */
+void
+H5TS_win32_process_exit(void)
+{
+
+    /* Windows uses a different thread local storage mechanism which does
+     * not support auto-freeing like pthreads' keys.
+     *
+     * This function is currently registered via atexit() and is called
+     * AFTER H5_term_library().
+     */
+
+    /* Clean up critical section resources (can't fail) */
+    DeleteCriticalSection(&H5_g.init_lock.CriticalSection);
+
+    /* Clean up per-process thread local storage */
+    TlsFree(H5TS_errstk_key_g);
+
+#ifdef H5_HAVE_CODESTACK
+    TlsFree(H5TS_funcstk_key_g);
+#endif /* H5_HAVE_CODESTACK */
+
+    return;
+} /* H5TS_win32_process_exit() */
+#endif /* H5_HAVE_WIN_THREADS */
+
+
+#ifdef H5_HAVE_WIN_THREADS
+/*--------------------------------------------------------------------------
+ * NAME
+ *    H5TS_win32_thread_exit
+ *
+ * RETURNS
+ *    SUCCEED/FAIL
+ *
+ * DESCRIPTION
+ *    Per-thread cleanup on Windows when using Win32 threads.
+ *
+ *--------------------------------------------------------------------------
+ */
+herr_t
+H5TS_win32_thread_exit(void)
+{
+    LPVOID lpvData;
+    herr_t ret_value = SUCCEED;
+
+    /* Windows uses a different thread local storage mechanism which does
+     * not support auto-freeing like pthreads' keys.
+     *
+     * WARNING: Do NOT use C standard library functions here.
+     * CRT functions are not allowed in DllMain, which is where this code
+     * is used.
+     */
+
+    /* Clean up per-thread thread local storage */
+    lpvData = TlsGetValue(H5TS_errstk_key_g);
+    if(lpvData)
+        LocalFree((HLOCAL)lpvData);
+
+#ifdef H5_HAVE_CODESTACK
+    lpvData = TlsGetValue(H5TS_funcstk_key_g);
+    if(lpvData)
+        LocalFree((HLOCAL)lpvData);
+#endif /* H5_HAVE_CODESTACK */
+
+    return ret_value;
+} /* H5TS_win32_thread_exit() */
+#endif /* H5_HAVE_WIN_THREADS */
+
+
 /*--------------------------------------------------------------------------
  * NAME
  *    H5TS_create_thread
@@ -399,16 +495,17 @@ H5TS_create_thread(void *func, H5TS_attr_t *attr, void *udata)
 
 #ifdef  H5_HAVE_WIN_THREADS 
 
-    /* When calling C runtime functions, you have to use _beginthread or
+    /* When calling C runtime functions, you should use _beginthread or
      * _beginthreadex instead of CreateThread.  Threads created with
-     * CreateThread risk being killed in low-memory situations.
-     * We use _beginthread instead of _begintheadex because the latter
-     * requires a stdcall function (and we don't need the more advanced
-     * features it exposes).
+     * CreateThread risk being killed in low-memory situations. Since we
+     * only create threads in our test code, this is unlikely to be an issue
+     * and we'll use the easier-to-deal-with CreateThread for now.
      *
-     * NOTE: No error checks here!  ret_value will be -1L on errors.
+     * NOTE: _beginthread() auto-recycles its handle when execution completes
+     *       so you can't wait on it, making it unsuitable for the existing
+     *       test code.
      */
-    ret_value = _beginthread(func, 0 /* stack size */, udata);
+    ret_value = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE)func, udata, 0, NULL);
 
 #else /* H5_HAVE_WIN_THREADS */
 
diff --git a/src/H5TSprivate.h b/src/H5TSprivate.h
index 7f55f4f..cc22f96 100644
--- a/src/H5TSprivate.h
+++ b/src/H5TSprivate.h
@@ -51,6 +51,7 @@ typedef INIT_ONCE H5TS_once_t;
 /* not used on windows side, but need to be defined to something */
 #define H5TS_SCOPE_SYSTEM 0
 #define H5TS_SCOPE_PROCESS 0
+#define H5TS_CALL_CONV WINAPI
 
 /* Functions */
 #define H5TS_get_thread_local_value(key)	TlsGetValue( key )
@@ -63,8 +64,13 @@ typedef INIT_ONCE H5TS_once_t;
 #define H5TS_mutex_lock_simple(mutex) EnterCriticalSection(mutex)
 #define H5TS_mutex_unlock_simple(mutex) LeaveCriticalSection(mutex)
 
-H5_DLL BOOL CALLBACK 
-H5TS_win32_first_thread_init(PINIT_ONCE InitOnce, PVOID Parameter, PVOID *lpContex);
+/* Functions called from DllMain */
+H5_DLL BOOL CALLBACK H5TS_win32_process_enter(PINIT_ONCE InitOnce, PVOID Parameter, PVOID *lpContex);
+H5_DLL void H5TS_win32_process_exit(void);
+H5_DLL herr_t H5TS_win32_thread_enter(void);
+H5_DLL herr_t H5TS_win32_thread_exit(void);
+
+
 
 #else /* H5_HAVE_WIN_THREADS */
 
@@ -86,6 +92,7 @@ typedef pthread_once_t H5TS_once_t;
 /* Scope Definitions */
 #define H5TS_SCOPE_SYSTEM PTHREAD_SCOPE_SYSTEM
 #define H5TS_SCOPE_PROCESS PTHREAD_SCOPE_PROCESS
+#define H5TS_CALL_CONV /* unused - Windows only */
 
 /* Functions */
 #define H5TS_get_thread_local_value(key)	pthread_getspecific( key )
@@ -117,8 +124,6 @@ H5_DLL herr_t H5TS_cancel_count_inc(void);
 H5_DLL herr_t H5TS_cancel_count_dec(void);
 H5_DLL H5TS_thread_t H5TS_create_thread(void * func, H5TS_attr_t * attr, void *udata);
 
-
-
 #if defined c_plusplus || defined __cplusplus
 }
 #endif	/* c_plusplus || __cplusplus */
diff --git a/src/H5private.h b/src/H5private.h
index d1dcd84..125e20c 100644
--- a/src/H5private.h
+++ b/src/H5private.h
@@ -1735,7 +1735,7 @@ typedef struct H5_api_struct {
 
 /* Macro for first thread initialization */
 #ifdef H5_HAVE_WIN_THREADS
-#define H5_FIRST_THREAD_INIT InitOnceExecuteOnce(&H5TS_first_init_g, H5TS_win32_first_thread_init, NULL, NULL);
+#define H5_FIRST_THREAD_INIT InitOnceExecuteOnce(&H5TS_first_init_g, H5TS_win32_process_enter, NULL, NULL);
 #else
 #define H5_FIRST_THREAD_INIT pthread_once(&H5TS_first_init_g, H5TS_pthread_first_thread_init);
 #endif