From e7b6ca156fc567d75da83409c64a4bccfbd98913 Mon Sep 17 00:00:00 2001 From: Nick Wellnhofer Date: Mon, 18 Sep 2023 13:25:06 +0200 Subject: [PATCH] globals: Rework global state destruction on Windows If DllMain is used, rely on it working as expected. The old code seemed to attempt to free global state of other threads if, for some reason, the DllMain mechanism didn't work. In a static build, register a destructor with RegisterWaitForSingleObject. Make public functions xmlGetGlobalState and xmlInitializeGlobalState no-ops. Move initialization and registration of global state objects to xmlInitGlobalState. Lookup global state with xmlGetThreadLocalStorage which can be inlined nicely. Also cleanup global state when using TLS. xmlLastError must be reset. --- globals.c | 317 ++++++++++++++++---------------------- include/libxml/globals.h | 2 +- include/private/threads.h | 3 - 3 files changed, 131 insertions(+), 191 deletions(-) diff --git a/globals.c b/globals.c index b663e609..a03a6aae 100644 --- a/globals.c +++ b/globals.c @@ -43,16 +43,17 @@ static xmlMutex xmlThrDefMutex; type gs_##name; struct _xmlGlobalState { +#if defined(HAVE_WIN32_THREADS) && \ + defined(LIBXML_STATIC) && !defined(LIBXML_STATIC_FOR_DLL) + void *threadHandle; + void *waitHandle; +#endif + #define XML_OP XML_DECLARE_MEMBER XML_GLOBALS #undef XML_OP }; -#ifdef LIBXML_THREAD_ENABLED -static void -xmlFreeGlobalState(void *state); -#endif - #ifdef HAVE_POSIX_THREADS /* @@ -81,42 +82,15 @@ static __declspec(thread) int tlstate_inited = 0; static DWORD globalkey = TLS_OUT_OF_INDEXES; -#if defined(LIBXML_STATIC) && !defined(LIBXML_STATIC_FOR_DLL) - -typedef struct _xmlGlobalStateCleanupHelperParams { - HANDLE thread; - void *memory; -} xmlGlobalStateCleanupHelperParams; - -static void -xmlGlobalStateCleanupHelper(void *p) -{ - xmlGlobalStateCleanupHelperParams *params = - (xmlGlobalStateCleanupHelperParams *) p; - WaitForSingleObject(params->thread, INFINITE); - CloseHandle(params->thread); - xmlFreeGlobalState(params->memory); - free(params); - _endthread(); -} - -#else /* LIBXML_STATIC && !LIBXML_STATIC_FOR_DLL */ - -typedef struct _xmlGlobalStateCleanupHelperParams { - void *memory; - struct _xmlGlobalStateCleanupHelperParams *prev; - struct _xmlGlobalStateCleanupHelperParams *next; -} xmlGlobalStateCleanupHelperParams; - -static xmlGlobalStateCleanupHelperParams *cleanup_helpers_head = NULL; -static CRITICAL_SECTION cleanup_helpers_cs; - -#endif /* LIBXMLSTATIC && !LIBXML_STATIC_FOR_DLL */ - #endif /* HAVE_COMPILER_TLS */ #endif /* HAVE_WIN32_THREADS */ +#ifdef LIBXML_THREAD_ENABLED +static void +xmlFreeGlobalState(void *state); +#endif + /************************************************************************ * * * All the user accessible global variables of the library * @@ -549,9 +523,6 @@ void xmlInitGlobalsInternal(void) { pthread_key_create(&globalkey, xmlFreeGlobalState); #elif defined(HAVE_WIN32_THREADS) #if !defined(HAVE_COMPILER_TLS) -#if !defined(LIBXML_STATIC) || defined(LIBXML_STATIC_FOR_DLL) - InitializeCriticalSection(&cleanup_helpers_cs); -#endif globalkey = TlsAlloc(); #endif #endif @@ -587,27 +558,9 @@ void xmlCleanupGlobalsInternal(void) { #elif defined(HAVE_WIN32_THREADS) #if !defined(HAVE_COMPILER_TLS) if (globalkey != TLS_OUT_OF_INDEXES) { -#if !defined(LIBXML_STATIC) || defined(LIBXML_STATIC_FOR_DLL) - xmlGlobalStateCleanupHelperParams *p; - - EnterCriticalSection(&cleanup_helpers_cs); - p = cleanup_helpers_head; - while (p != NULL) { - xmlGlobalStateCleanupHelperParams *temp = p; - - p = p->next; - xmlFreeGlobalState(temp->memory); - free(temp); - } - cleanup_helpers_head = 0; - LeaveCriticalSection(&cleanup_helpers_cs); -#endif TlsFree(globalkey); globalkey = TLS_OUT_OF_INDEXES; } -#if !defined(LIBXML_STATIC) || defined(LIBXML_STATIC_FOR_DLL) - DeleteCriticalSection(&cleanup_helpers_cs); -#endif #endif #endif } @@ -616,14 +569,80 @@ void xmlCleanupGlobalsInternal(void) { * xmlInitializeGlobalState: * @gs: a pointer to a newly allocated global state * - * DEPRECATED: Internal function, do not use. - * - * xmlInitializeGlobalState() initialize a global state with all the - * default values of the library. + * DEPRECATED: No-op. */ void -xmlInitializeGlobalState(xmlGlobalStatePtr gs) +xmlInitializeGlobalState(xmlGlobalStatePtr gs ATTRIBUTE_UNUSED) { +} + +/** + * xmlGetGlobalState: + * + * DEPRECATED: Always returns NULL. + */ +xmlGlobalStatePtr +xmlGetGlobalState(void) +{ + return(NULL); +} + +#ifdef LIBXML_THREAD_ENABLED +/** + * xmlFreeGlobalState: + * @state: a thread global state + * + * xmlFreeGlobalState() is called when a thread terminates with a non-NULL + * global state. It is is used here to reclaim memory resources. + */ +static void +xmlFreeGlobalState(void *state) +{ + xmlGlobalState *gs = (xmlGlobalState *) state; + + /* free any memory allocated in the thread's xmlLastError */ + xmlResetError(&(gs->gs_xmlLastError)); +#if !defined(HAVE_WIN32_THREADS) || !defined(HAVE_COMPILER_TLS) + free(state); +#endif +} + +#if defined(HAVE_WIN32_THREADS) && \ + defined(LIBXML_STATIC) && !defined(LIBXML_STATIC_FOR_DLL) +static void WINAPI +xmlGlobalStateDtor(void *ctxt, unsigned char timedOut ATTRIBUTE_UNUSED) { + xmlGlobalStatePtr gs = ctxt; + + UnregisterWait(gs->waitHandle); + CloseHandle(gs->threadHandle); + xmlFreeGlobalState(gs); +} + +static int +xmlRegisterGlobalStateDtor(xmlGlobalState *gs) { + void *processHandle = GetCurrentProcess(); + void *threadHandle; + void *waitHandle; + + if (DuplicateHandle(processHandle, GetCurrentThread(), processHandle, + &threadHandle, 0, FALSE, DUPLICATE_SAME_ACCESS) == 0) { + return(-1); + } + + if (RegisterWaitForSingleObject(&waitHandle, threadHandle, + xmlGlobalStateDtor, gs, INFINITE, WT_EXECUTEONLYONCE) == 0) { + CloseHandle(threadHandle); + return(-1); + } + + gs->threadHandle = threadHandle; + gs->waitHandle = waitHandle; + return(0); +} +#endif /* LIBXML_STATIC */ + +static void +xmlInitGlobalState(xmlGlobalStatePtr gs) { xmlMutexLock(&xmlThrDefMutex); #if defined(LIBXML_HTML_ENABLED) && defined(LIBXML_LEGACY_ENABLED) && defined(LIBXML_SAX1_ENABLED) @@ -676,24 +695,17 @@ xmlInitializeGlobalState(xmlGlobalStatePtr gs) memset(&gs->gs_xmlLastError, 0, sizeof(xmlError)); xmlMutexUnlock(&xmlThrDefMutex); -} -#ifdef LIBXML_THREAD_ENABLED -/** - * xmlFreeGlobalState: - * @state: a thread global state - * - * xmlFreeGlobalState() is called when a thread terminates with a non-NULL - * global state. It is is used here to reclaim memory resources. - */ -static void -xmlFreeGlobalState(void *state) -{ - xmlGlobalState *gs = (xmlGlobalState *) state; - - /* free any memory allocated in the thread's xmlLastError */ - xmlResetError(&(gs->gs_xmlLastError)); - free(state); +#ifdef HAVE_POSIX_THREADS + pthread_setspecific(globalkey, gs); +#elif defined HAVE_WIN32_THREADS +#ifndef HAVE_COMPILER_TLS + TlsSetValue(globalkey, gs); +#endif +#if defined(LIBXML_STATIC) && !defined(LIBXML_STATIC_FOR_DLL) + xmlRegisterGlobalStateDtor(gs); +#endif +#endif } /** @@ -711,16 +723,40 @@ xmlNewGlobalState(void) xmlGlobalState *gs; gs = malloc(sizeof(xmlGlobalState)); - if (gs == NULL) { - xmlGenericError(xmlGenericErrorContext, - "xmlGetGlobalState: out of memory\n"); + if (gs == NULL) return (NULL); - } memset(gs, 0, sizeof(xmlGlobalState)); - xmlInitializeGlobalState(gs); + xmlInitGlobalState(gs); return (gs); } + +static xmlGlobalStatePtr +xmlGetThreadLocalStorage(void) { +#ifdef HAVE_POSIX_THREADS + xmlGlobalState *gs; + gs = (xmlGlobalState *) pthread_getspecific(globalkey); + if (gs == NULL) + gs = xmlNewGlobalState(); + return (gs); +#elif defined HAVE_WIN32_THREADS +#if defined(HAVE_COMPILER_TLS) + if (!tlstate_inited) { + tlstate_inited = 1; + xmlInitGlobalState(&tlstate); + } + return &tlstate; +#else /* HAVE_COMPILER_TLS */ + xmlGlobalState *gs; + gs = (xmlGlobalState *) TlsGetValue(globalkey); + if (gs == NULL) + gs = xmlNewGlobalState(); + return (gs); +#endif /* HAVE_COMPILER_TLS */ +#else + return (NULL); +#endif +} #endif /* LIBXML_THREAD_ENABLED */ /** @@ -743,91 +779,11 @@ xmlNewGlobalState(void) */ int xmlCheckThreadLocalStorage(void) { - if (IS_MAIN_THREAD) - return(0); - return((xmlGetGlobalState() == NULL) ? -1 : 0); -} - -/** - * xmlGetGlobalState: - * - * DEPRECATED: Internal function, do not use. - * - * xmlGetGlobalState() is called to retrieve the global state for a thread. - * - * Returns the thread global state or NULL in case of error - */ -xmlGlobalStatePtr -xmlGetGlobalState(void) -{ -#ifdef HAVE_POSIX_THREADS - xmlGlobalState *globalval; - - if ((globalval = (xmlGlobalState *) - pthread_getspecific(globalkey)) == NULL) { - xmlGlobalState *tsd = xmlNewGlobalState(); - if (tsd == NULL) - return(NULL); - - pthread_setspecific(globalkey, tsd); - return (tsd); - } - return (globalval); -#elif defined HAVE_WIN32_THREADS -#if defined(HAVE_COMPILER_TLS) - if (!tlstate_inited) { - tlstate_inited = 1; - xmlInitializeGlobalState(&tlstate); - } - return &tlstate; -#else /* HAVE_COMPILER_TLS */ - xmlGlobalState *globalval; - xmlGlobalStateCleanupHelperParams *p; -#if defined(LIBXML_STATIC) && !defined(LIBXML_STATIC_FOR_DLL) - globalval = (xmlGlobalState *) TlsGetValue(globalkey); -#else - p = (xmlGlobalStateCleanupHelperParams *) TlsGetValue(globalkey); - globalval = (xmlGlobalState *) (p ? p->memory : NULL); -#endif - if (globalval == NULL) { - xmlGlobalState *tsd = xmlNewGlobalState(); - - if (tsd == NULL) - return(NULL); - p = (xmlGlobalStateCleanupHelperParams *) - malloc(sizeof(xmlGlobalStateCleanupHelperParams)); - if (p == NULL) { - xmlGenericError(xmlGenericErrorContext, - "xmlGetGlobalState: out of memory\n"); - xmlFreeGlobalState(tsd); - return(NULL); - } - p->memory = tsd; -#if defined(LIBXML_STATIC) && !defined(LIBXML_STATIC_FOR_DLL) - DuplicateHandle(GetCurrentProcess(), GetCurrentThread(), - GetCurrentProcess(), &p->thread, 0, TRUE, - DUPLICATE_SAME_ACCESS); - TlsSetValue(globalkey, tsd); - _beginthread(xmlGlobalStateCleanupHelper, 0, p); -#else - EnterCriticalSection(&cleanup_helpers_cs); - if (cleanup_helpers_head != NULL) { - cleanup_helpers_head->prev = p; - } - p->next = cleanup_helpers_head; - p->prev = NULL; - cleanup_helpers_head = p; - TlsSetValue(globalkey, p); - LeaveCriticalSection(&cleanup_helpers_cs); -#endif - - return (tsd); - } - return (globalval); -#endif /* HAVE_COMPILER_TLS */ -#else - return (NULL); +#ifdef LIBXML_THREAD_ENABLED + if ((!xmlIsMainThread()) && (xmlGetThreadLocalStorage() == NULL)) + return(-1); #endif + return(0); } /** @@ -842,7 +798,7 @@ xmlGetGlobalState(void) * Returns TRUE always */ #ifdef HAVE_POSIX_THREADS -#elif defined(HAVE_WIN32_THREADS) && !defined(HAVE_COMPILER_TLS) && (!defined(LIBXML_STATIC) || defined(LIBXML_STATIC_FOR_DLL)) +#elif defined(HAVE_WIN32_THREADS) && (!defined(LIBXML_STATIC) || defined(LIBXML_STATIC_FOR_DLL)) #if defined(LIBXML_STATIC_FOR_DLL) int xmlDllMain(ATTRIBUTE_UNUSED void *hinstDLL, unsigned long fdwReason, @@ -866,26 +822,13 @@ DllMain(ATTRIBUTE_UNUSED HINSTANCE hinstDLL, DWORD fdwReason, switch (fdwReason) { case DLL_THREAD_DETACH: if (globalkey != TLS_OUT_OF_INDEXES) { - xmlGlobalState *globalval = NULL; - xmlGlobalStateCleanupHelperParams *p = - (xmlGlobalStateCleanupHelperParams *) - TlsGetValue(globalkey); - globalval = (xmlGlobalState *) (p ? p->memory : NULL); + xmlGlobalState *globalval; + + globalval = (xmlGlobalState *) TlsGetValue(globalkey); if (globalval) { xmlFreeGlobalState(globalval); TlsSetValue(globalkey, NULL); } - if (p) { - EnterCriticalSection(&cleanup_helpers_cs); - if (p == cleanup_helpers_head) - cleanup_helpers_head = p->next; - else - p->prev->next = p->next; - if (p->next != NULL) - p->next->prev = p->prev; - LeaveCriticalSection(&cleanup_helpers_cs); - free(p); - } } break; } @@ -1138,7 +1081,7 @@ xmlThrDefOutputBufferCreateFilenameDefault(xmlOutputBufferCreateFilenameFunc fun if (IS_MAIN_THREAD) \ return (&name); \ else \ - return (&xmlGetGlobalState()->gs_##name); \ + return (&xmlGetThreadLocalStorage()->gs_##name); \ } #define XML_OP XML_DEFINE_GLOBAL_WRAPPER diff --git a/include/libxml/globals.h b/include/libxml/globals.h index 85c10574..d49f11db 100644 --- a/include/libxml/globals.h +++ b/include/libxml/globals.h @@ -122,7 +122,7 @@ XMLPUBFUN xmlParserInputBufferCreateFilenameFunc /** DOC_DISABLE */ #if defined(LIBXML_THREAD_ENABLED) && defined(_WIN32) && \ - !defined(HAVE_COMPILER_TLS) && defined(LIBXML_STATIC_FOR_DLL) + defined(LIBXML_STATIC_FOR_DLL) int xmlDllMain(void *hinstDLL, unsigned long fdwReason, void *lpvReserved); diff --git a/include/private/threads.h b/include/private/threads.h index 4c5948c0..473bc7c0 100644 --- a/include/private/threads.h +++ b/include/private/threads.h @@ -10,9 +10,6 @@ #elif defined(_WIN32) #define WIN32_LEAN_AND_MEAN #include - #ifndef HAVE_COMPILER_TLS - #include - #endif #define HAVE_WIN32_THREADS #endif #endif