From 6eedad45bcb621c6d26aafa4ec47d37cdbc1204e Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Wed, 11 Jan 2017 21:55:35 +0100 Subject: Fix Unix thread shutdown There was a problem with the secondary thread shutdown on Unix. First, when the TlsObjectDestructor is called, it is passed the key that was set using the pthread_setspecific before. But I was missing the fact that before the TlsObjectDestructor is called, the key is set to NULL. So we cannot use pthread_getspecific to get the key anymore and we can only use the value passed as the TlsObjectDestructor parameter. The second part of the problem was that the at the time the TlsObjectDestructor is called, the tls_CurrentThread to which the key points was already removed from the TLS and so an attempt to access tls_CurrentThread results in creation of a fresh new tls_CurrentThread. And our checks that we have at a few places that verify that the argument passed to the TlsObjectDestructor was actually the current thread pointer cannot be done. The fix is to use __cxa_thread_atexit to register a destruction callback for the ThreadStore object and use it instead of the pthread callback functionality. --- buildscripts/buildvars-setup.sh | 4 -- src/Native/Runtime/unix/PalRedhawkUnix.cpp | 72 +++++++++++------------------- 2 files changed, 25 insertions(+), 51 deletions(-) diff --git a/buildscripts/buildvars-setup.sh b/buildscripts/buildvars-setup.sh index ead93b646..588bcd626 100755 --- a/buildscripts/buildvars-setup.sh +++ b/buildscripts/buildvars-setup.sh @@ -190,10 +190,6 @@ while [ "$1" != "" ]; do verbose) export __VerboseBuild=1 ;; - clang3.5) - export __ClangMajorVersion=3 - export __ClangMinorVersion=5 - ;; clang3.6) export __ClangMajorVersion=3 export __ClangMinorVersion=6 diff --git a/src/Native/Runtime/unix/PalRedhawkUnix.cpp b/src/Native/Runtime/unix/PalRedhawkUnix.cpp index 91c5842cc..47df6e2d9 100644 --- a/src/Native/Runtime/unix/PalRedhawkUnix.cpp +++ b/src/Native/Runtime/unix/PalRedhawkUnix.cpp @@ -158,9 +158,6 @@ static uint8_t g_helperPage[OS_PAGE_SIZE] __attribute__((aligned(OS_PAGE_SIZE))) // Mutex to make the FlushProcessWriteBuffersMutex thread safe pthread_mutex_t g_flushProcessWriteBuffersMutex; -// Key for the thread local storage of the attached thread pointer -static pthread_key_t g_threadKey; - extern bool PalQueryProcessorTopology(); bool InitializeFlushProcessWriteBuffers(); @@ -413,15 +410,6 @@ public: typedef UnixHandle ThreadUnixHandle; -// Destructor of the thread local object represented by the g_threadKey, -// called when a thread is shut down -void TlsObjectDestructor(void* data) -{ - ASSERT(data == pthread_getspecific(g_threadKey)); - - RuntimeThreadShutdown(data); -} - // The Redhawk PAL must be initialized before any of its exports can be called. Returns true for a successful // initialization and false on failure. REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalInit() @@ -449,11 +437,6 @@ REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalInit() return false; } #endif // !USE_PORTABLE_HELPERS - int status = pthread_key_create(&g_threadKey, TlsObjectDestructor); - if (status != 0) - { - return false; - } return true; } @@ -464,6 +447,28 @@ REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalHasCapability(PalCapability capability) return (g_dwPALCapabilities & (uint32_t)capability) == (uint32_t)capability; } +struct TlsDestructionMonitor +{ + void* m_thread = nullptr; + + void SetThread(void* thread) + { + m_thread = thread; + } + + ~TlsDestructionMonitor() + { + if (m_thread != nullptr) + { + RuntimeThreadShutdown(m_thread); + } + } +}; + +// This thread local object is used to detect thread shutdown. Its destructor +// is called when a thread is being shut down. +thread_local TlsDestructionMonitor tls_destructionMonitor; + // Attach thread to PAL. // It can be called multiple times for the same thread. // It fails fast if a different thread was already registered. @@ -471,16 +476,7 @@ REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalHasCapability(PalCapability capability) // thread - thread to attach extern "C" void PalAttachThread(void* thread) { - void* attachedThread = pthread_getspecific(g_threadKey); - - ASSERT_MSG(attachedThread == NULL, "PalAttachThread called multiple times for the same thread"); - - int status = pthread_setspecific(g_threadKey, thread); - if (status != 0) - { - ASSERT_UNCONDITIONALLY("PalAttachThread failed to store thread pointer in thread local storage"); - RhFailFast(); - } + tls_destructionMonitor.SetThread(thread); } // Detach thread from PAL. @@ -491,26 +487,8 @@ extern "C" void PalAttachThread(void* thread) // true if the thread was detached, false if there was no attached thread extern "C" bool PalDetachThread(void* thread) { - void* attachedThread = pthread_getspecific(g_threadKey); - - if (attachedThread == thread) - { - int status = pthread_setspecific(g_threadKey, NULL); - if (status != 0) - { - ASSERT_UNCONDITIONALLY("PalDetachThread failed to clear thread pointer in thread local storage"); - RhFailFast(); - } - return true; - } - - if (attachedThread != NULL) - { - ASSERT_UNCONDITIONALLY("PalDetachThread called with different thread pointer than PalAttachThread"); - RhFailFast(); - } - - return false; + UNREFERENCED_PARAMETER(thread); + return true; } REDHAWK_PALEXPORT unsigned int REDHAWK_PALAPI PalGetCurrentProcessorNumber() -- cgit v1.2.3 From 0893676f65845428cce02e71871b34ad7b676fd8 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Thu, 19 Jan 2017 22:22:57 +0100 Subject: Enable workaround for build tools that don't support thread_local Apple started to support thread_local in xcode starting from version 8.0, this workaround enables the thread shutdown detection to compile even on devices that don't have the latest xcode. --- src/Native/Runtime/startup.cpp | 5 +++++ src/Native/Runtime/unix/PalRedhawkUnix.cpp | 15 +++++++++++++++ src/Native/Runtime/unix/config.h.in | 2 ++ src/Native/Runtime/unix/configure.cmake | 9 +++++++++ 4 files changed, 31 insertions(+) diff --git a/src/Native/Runtime/startup.cpp b/src/Native/Runtime/startup.cpp index 8a7b0c56c..3a368191b 100644 --- a/src/Native/Runtime/startup.cpp +++ b/src/Native/Runtime/startup.cpp @@ -268,8 +268,13 @@ void __stdcall RuntimeThreadShutdown(void* thread) // that is made for the single thread that runs the final stages of orderly process // shutdown (i.e., the thread that delivers the DLL_PROCESS_DETACH notifications when the // process is being torn down via an ExitProcess call). +#if HAVE_THREAD_LOCAL + // If the current Unix platform doesn't support thread_local, we don't get the thread pointer + // as the parameter, we just get NULL, so we can check the thread validity only if the + // thread_local is supported UNREFERENCED_PARAMETER(thread); ASSERT((Thread*)thread == ThreadStore::GetCurrentThread()); +#endif if (!g_processShutdownHasStarted) { diff --git a/src/Native/Runtime/unix/PalRedhawkUnix.cpp b/src/Native/Runtime/unix/PalRedhawkUnix.cpp index 47df6e2d9..c9f8ea4a8 100644 --- a/src/Native/Runtime/unix/PalRedhawkUnix.cpp +++ b/src/Native/Runtime/unix/PalRedhawkUnix.cpp @@ -410,6 +410,11 @@ public: typedef UnixHandle ThreadUnixHandle; +#if !HAVE_THREAD_LOCAL +extern "C" int __cxa_thread_atexit(void (*)(void*), void*, void *); +extern "C" void *__dso_handle; +#endif + // The Redhawk PAL must be initialized before any of its exports can be called. Returns true for a successful // initialization and false on failure. REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalInit() @@ -438,6 +443,10 @@ REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalInit() } #endif // !USE_PORTABLE_HELPERS +#if !HAVE_THREAD_LOCAL + __cxa_thread_atexit(RuntimeThreadShutdown, NULL, &__dso_handle); +#endif + return true; } @@ -447,6 +456,8 @@ REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalHasCapability(PalCapability capability) return (g_dwPALCapabilities & (uint32_t)capability) == (uint32_t)capability; } +#if HAVE_THREAD_LOCAL + struct TlsDestructionMonitor { void* m_thread = nullptr; @@ -469,6 +480,8 @@ struct TlsDestructionMonitor // is called when a thread is being shut down. thread_local TlsDestructionMonitor tls_destructionMonitor; +#endif // HAVE_THREAD_LOCAL + // Attach thread to PAL. // It can be called multiple times for the same thread. // It fails fast if a different thread was already registered. @@ -476,7 +489,9 @@ thread_local TlsDestructionMonitor tls_destructionMonitor; // thread - thread to attach extern "C" void PalAttachThread(void* thread) { +#if HAVE_THREAD_LOCAL tls_destructionMonitor.SetThread(thread); +#endif } // Detach thread from PAL. diff --git a/src/Native/Runtime/unix/config.h.in b/src/Native/Runtime/unix/config.h.in index 085ceaa3a..ca5f5aa71 100644 --- a/src/Native/Runtime/unix/config.h.in +++ b/src/Native/Runtime/unix/config.h.in @@ -29,4 +29,6 @@ #cmakedefine01 HAVE_CLOCK_MONOTONIC_COARSE #cmakedefine01 HAVE_MACH_ABSOLUTE_TIME +#cmakedefine01 HAVE_THREAD_LOCAL + #endif diff --git a/src/Native/Runtime/unix/configure.cmake b/src/Native/Runtime/unix/configure.cmake index d58e1219e..a8fd29d18 100644 --- a/src/Native/Runtime/unix/configure.cmake +++ b/src/Native/Runtime/unix/configure.cmake @@ -105,4 +105,13 @@ int main() exit(ret); }" HAVE_MACH_ABSOLUTE_TIME) +check_cxx_source_compiles(" +thread_local int x; + +int main(int argc, char **argv) +{ + x = 1; + return 0; +}" HAVE_THREAD_LOCAL) + configure_file(${CMAKE_CURRENT_LIST_DIR}/config.h.in ${CMAKE_CURRENT_BINARY_DIR}/config.h) -- cgit v1.2.3