From b17ef126a75779f00ee4fef70a2b590b2c19fb4e Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Fri, 4 Apr 2014 15:51:30 -0500 Subject: [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) --- CMakeLists.txt | 9 ++- src/H5.c | 73 +++++++++++++++++- src/H5CS.c | 10 ++- src/H5E.c | 8 +- src/H5TS.c | 225 ++++++++++++++++++++++++++++++++++++++---------------- src/H5TSprivate.h | 13 +++- src/H5private.h | 2 +- 7 files changed, 260 insertions(+), 80 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index efa9ba6..106c59c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -730,14 +730,17 @@ if (WIN32) option (HDF5_ENABLE_THREADSAFE "Enable thread-safety" OFF) if (HDF5_ENABLE_THREADSAFE) # check for unsupported options + if (H5_BUILT_AS_STATIC_LIB) + message (FATAL_ERROR " **** thread-safety option not supported with static library **** ") + endif (H5_BUILT_AS_STATIC_LIB) if (HDF5_ENABLE_PARALLEL) - message (FATAL_ERROR " **** parallel and thread-safety options are mutually exclusive **** ") + message (FATAL_ERROR " **** parallel and thread-safety options are not supported **** ") endif (HDF5_ENABLE_PARALLEL) if (HDF5_BUILD_FORTRAN) - message (FATAL_ERROR " **** Fortran and thread-safety options are mutually exclusive **** ") + message (FATAL_ERROR " **** Fortran and thread-safety options are not supported **** ") endif (HDF5_BUILD_FORTRAN) if (HDF5_BUILD_CPP_LIB) - message (FATAL_ERROR " **** C++ and thread-safety options are mutually exclusive **** ") + message (FATAL_ERROR " **** C++ and thread-safety options are not supported **** ") endif (HDF5_BUILD_CPP_LIB) set (H5_HAVE_THREADSAFE 1) if (H5_HAVE_IOEO) 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 -- cgit v0.12