# HG changeset patch # User dholmes # Date 1462942948 14400 # Node ID 6b194cfc1557462cb36303ad40c99b1760c1adf9 # Parent 96e35aced4ef5750b214f5911e73cb9c4bd5b412 8154715: Missing destructor and/or TLS clearing calls for terminating threads Summary: clear TLS on return from thread->run() unless already done. Prohibit asynchronous thread deletion. Reviewed-by: stefank, sspitsyn Contributed-by: Brian Gardner diff -r 96e35aced4ef -r 6b194cfc1557 hotspot/src/os/aix/vm/os_aix.cpp --- a/hotspot/src/os/aix/vm/os_aix.cpp Wed May 11 00:40:59 2016 +0000 +++ b/hotspot/src/os/aix/vm/os_aix.cpp Wed May 11 01:02:28 2016 -0400 @@ -777,7 +777,7 @@ // create new thread // Thread start routine for all newly created threads -static void *java_start(Thread *thread) { +static void *thread_native_entry(Thread *thread) { // find out my own stack dimensions { @@ -838,6 +838,15 @@ log_info(os, thread)("Thread finished (tid: " UINTX_FORMAT ", kernel thread id: " UINTX_FORMAT ").", os::current_thread_id(), (uintx) kernel_thread_id); + // If a thread has not deleted itself ("delete this") as part of its + // termination sequence, we have to ensure thread-local-storage is + // cleared before we actually terminate. No threads should ever be + // deleted asynchronously with respect to their termination. + if (Thread::current_or_null_safe() != NULL) { + assert(Thread::current_or_null_safe() == thread, "current thread is wrong"); + thread->clear_thread_current(); + } + return 0; } @@ -902,7 +911,7 @@ pthread_attr_setstacksize(&attr, stack_size); pthread_t tid; - int ret = pthread_create(&tid, &attr, (void* (*)(void*)) java_start, thread); + int ret = pthread_create(&tid, &attr, (void* (*)(void*)) thread_native_entry, thread); char buf[64]; @@ -993,11 +1002,14 @@ void os::free_thread(OSThread* osthread) { assert(osthread != NULL, "osthread not set"); - if (Thread::current()->osthread() == osthread) { - // Restore caller's signal mask - sigset_t sigmask = osthread->caller_sigmask(); - pthread_sigmask(SIG_SETMASK, &sigmask, NULL); - } + // We are told to free resources of the argument thread, + // but we can only really operate on the current thread. + assert(Thread::current()->osthread() == osthread, + "os::free_thread but not current thread"); + + // Restore caller's signal mask + sigset_t sigmask = osthread->caller_sigmask(); + pthread_sigmask(SIG_SETMASK, &sigmask, NULL); delete osthread; } diff -r 96e35aced4ef -r 6b194cfc1557 hotspot/src/os/bsd/vm/os_bsd.cpp --- a/hotspot/src/os/bsd/vm/os_bsd.cpp Wed May 11 00:40:59 2016 +0000 +++ b/hotspot/src/os/bsd/vm/os_bsd.cpp Wed May 11 01:02:28 2016 -0400 @@ -665,7 +665,7 @@ #endif // Thread start routine for all newly created threads -static void *java_start(Thread *thread) { +static void *thread_native_entry(Thread *thread) { // Try to randomize the cache line index of hot stack frames. // This helps when threads of the same stack traces evict each other's // cache lines. The threads can be either from the same JVM instance, or @@ -723,6 +723,15 @@ log_info(os, thread)("Thread finished (tid: " UINTX_FORMAT ", pthread id: " UINTX_FORMAT ").", os::current_thread_id(), (uintx) pthread_self()); + // If a thread has not deleted itself ("delete this") as part of its + // termination sequence, we have to ensure thread-local-storage is + // cleared before we actually terminate. No threads should ever be + // deleted asynchronously with respect to their termination. + if (Thread::current_or_null_safe() != NULL) { + assert(Thread::current_or_null_safe() == thread, "current thread is wrong"); + thread->clear_thread_current(); + } + return 0; } @@ -781,7 +790,7 @@ { pthread_t tid; - int ret = pthread_create(&tid, &attr, (void* (*)(void*)) java_start, thread); + int ret = pthread_create(&tid, &attr, (void* (*)(void*)) thread_native_entry, thread); char buf[64]; if (ret == 0) { @@ -889,11 +898,14 @@ void os::free_thread(OSThread* osthread) { assert(osthread != NULL, "osthread not set"); - if (Thread::current()->osthread() == osthread) { - // Restore caller's signal mask - sigset_t sigmask = osthread->caller_sigmask(); - pthread_sigmask(SIG_SETMASK, &sigmask, NULL); - } + // We are told to free resources of the argument thread, + // but we can only really operate on the current thread. + assert(Thread::current()->osthread() == osthread, + "os::free_thread but not current thread"); + + // Restore caller's signal mask + sigset_t sigmask = osthread->caller_sigmask(); + pthread_sigmask(SIG_SETMASK, &sigmask, NULL); delete osthread; } diff -r 96e35aced4ef -r 6b194cfc1557 hotspot/src/os/linux/vm/os_linux.cpp --- a/hotspot/src/os/linux/vm/os_linux.cpp Wed May 11 00:40:59 2016 +0000 +++ b/hotspot/src/os/linux/vm/os_linux.cpp Wed May 11 01:02:28 2016 -0400 @@ -638,7 +638,7 @@ // create new thread // Thread start routine for all newly created threads -static void *java_start(Thread *thread) { +static void *thread_native_entry(Thread *thread) { // Try to randomize the cache line index of hot stack frames. // This helps when threads of the same stack traces evict each other's // cache lines. The threads can be either from the same JVM instance, or @@ -690,6 +690,15 @@ log_info(os, thread)("Thread finished (tid: " UINTX_FORMAT ", pthread id: " UINTX_FORMAT ").", os::current_thread_id(), (uintx) pthread_self()); + // If a thread has not deleted itself ("delete this") as part of its + // termination sequence, we have to ensure thread-local-storage is + // cleared before we actually terminate. No threads should ever be + // deleted asynchronously with respect to their termination. + if (Thread::current_or_null_safe() != NULL) { + assert(Thread::current_or_null_safe() == thread, "current thread is wrong"); + thread->clear_thread_current(); + } + return 0; } @@ -753,7 +762,7 @@ { pthread_t tid; - int ret = pthread_create(&tid, &attr, (void* (*)(void*)) java_start, thread); + int ret = pthread_create(&tid, &attr, (void* (*)(void*)) thread_native_entry, thread); char buf[64]; if (ret == 0) { @@ -881,18 +890,21 @@ void os::free_thread(OSThread* osthread) { assert(osthread != NULL, "osthread not set"); - if (Thread::current()->osthread() == osthread) { + // We are told to free resources of the argument thread, + // but we can only really operate on the current thread. + assert(Thread::current()->osthread() == osthread, + "os::free_thread but not current thread"); + #ifdef ASSERT - sigset_t current; - sigemptyset(¤t); - pthread_sigmask(SIG_SETMASK, NULL, ¤t); - assert(!sigismember(¤t, SR_signum), "SR signal should not be blocked!"); + sigset_t current; + sigemptyset(¤t); + pthread_sigmask(SIG_SETMASK, NULL, ¤t); + assert(!sigismember(¤t, SR_signum), "SR signal should not be blocked!"); #endif - // Restore caller's signal mask - sigset_t sigmask = osthread->caller_sigmask(); - pthread_sigmask(SIG_SETMASK, &sigmask, NULL); - } + // Restore caller's signal mask + sigset_t sigmask = osthread->caller_sigmask(); + pthread_sigmask(SIG_SETMASK, &sigmask, NULL); delete osthread; } diff -r 96e35aced4ef -r 6b194cfc1557 hotspot/src/os/solaris/vm/os_solaris.cpp --- a/hotspot/src/os/solaris/vm/os_solaris.cpp Wed May 11 00:40:59 2016 +0000 +++ b/hotspot/src/os/solaris/vm/os_solaris.cpp Wed May 11 01:02:28 2016 -0400 @@ -725,8 +725,8 @@ static thread_t main_thread; -// Thread start routine for all new Java threads -extern "C" void* java_start(void* thread_addr) { +// Thread start routine for all newly created threads +extern "C" void* thread_native_entry(void* thread_addr) { // Try to randomize the cache line index of hot stack frames. // This helps when threads of the same stack traces evict each other's // cache lines. The threads can be either from the same JVM instance, or @@ -796,6 +796,15 @@ log_info(os, thread)("Thread finished (tid: " UINTX_FORMAT ").", os::current_thread_id()); + // If a thread has not deleted itself ("delete this") as part of its + // termination sequence, we have to ensure thread-local-storage is + // cleared before we actually terminate. No threads should ever be + // deleted asynchronously with respect to their termination. + if (Thread::current_or_null_safe() != NULL) { + assert(Thread::current_or_null_safe() == thread, "current thread is wrong"); + thread->clear_thread_current(); + } + if (UseDetachedThreads) { thr_exit(NULL); ShouldNotReachHere(); @@ -1009,7 +1018,7 @@ osthread->set_lwp_id(-1); osthread->set_thread_id(-1); - status = thr_create(NULL, stack_size, java_start, thread, flags, &tid); + status = thr_create(NULL, stack_size, thread_native_entry, thread, flags, &tid); char buf[64]; if (status == 0) { @@ -1221,18 +1230,15 @@ void os::free_thread(OSThread* osthread) { assert(osthread != NULL, "os::free_thread but osthread not set"); - // We are told to free resources of the argument thread, // but we can only really operate on the current thread. - // The main thread must take the VMThread down synchronously - // before the main thread exits and frees up CodeHeap - guarantee((Thread::current()->osthread() == osthread - || (osthread == VMThread::vm_thread()->osthread())), "os::free_thread but not current thread"); - if (Thread::current()->osthread() == osthread) { - // Restore caller's signal mask - sigset_t sigmask = osthread->caller_sigmask(); - pthread_sigmask(SIG_SETMASK, &sigmask, NULL); - } + assert(Thread::current()->osthread() == osthread, + "os::free_thread but not current thread"); + + // Restore caller's signal mask + sigset_t sigmask = osthread->caller_sigmask(); + pthread_sigmask(SIG_SETMASK, &sigmask, NULL); + delete osthread; } diff -r 96e35aced4ef -r 6b194cfc1557 hotspot/src/os/windows/vm/os_windows.cpp --- a/hotspot/src/os/windows/vm/os_windows.cpp Wed May 11 00:40:59 2016 +0000 +++ b/hotspot/src/os/windows/vm/os_windows.cpp Wed May 11 01:02:28 2016 -0400 @@ -409,8 +409,8 @@ LONG WINAPI topLevelExceptionFilter(struct _EXCEPTION_POINTERS* exceptionInfo); -// Thread start routine for all new Java threads -static unsigned __stdcall java_start(Thread* thread) { +// Thread start routine for all newly created threads +static unsigned __stdcall thread_native_entry(Thread* thread) { // Try to randomize the cache line index of hot stack frames. // This helps when threads of the same stack traces evict each other's // cache lines. The threads can be either from the same JVM instance, or @@ -459,6 +459,15 @@ Atomic::dec_ptr((intptr_t*)&os::win32::_os_thread_count); } + // If a thread has not deleted itself ("delete this") as part of its + // termination sequence, we have to ensure thread-local-storage is + // cleared before we actually terminate. No threads should ever be + // deleted asynchronously with respect to their termination. + if (Thread::current_or_null_safe() != NULL) { + assert(Thread::current_or_null_safe() == thread, "current thread is wrong"); + thread->clear_thread_current(); + } + // Thread must not return from exit_process_or_thread(), but if it does, // let it proceed to exit normally return (unsigned)os::win32::exit_process_or_thread(os::win32::EPT_THREAD, res); @@ -631,7 +640,7 @@ HANDLE thread_handle = (HANDLE)_beginthreadex(NULL, (unsigned)stack_size, - (unsigned (__stdcall *)(void*)) java_start, + (unsigned (__stdcall *)(void*)) thread_native_entry, thread, initflag, &thread_id); @@ -670,6 +679,12 @@ // Free Win32 resources related to the OSThread void os::free_thread(OSThread* osthread) { assert(osthread != NULL, "osthread not set"); + + // We are told to free resources of the argument thread, + // but we can only really operate on the current thread. + assert(Thread::current()->osthread() == osthread, + "os::free_thread but not current thread"); + CloseHandle(osthread->thread_handle()); CloseHandle(osthread->interrupt_event()); delete osthread; diff -r 96e35aced4ef -r 6b194cfc1557 hotspot/src/os/windows/vm/os_windows.hpp --- a/hotspot/src/os/windows/vm/os_windows.hpp Wed May 11 00:40:59 2016 +0000 +++ b/hotspot/src/os/windows/vm/os_windows.hpp Wed May 11 01:02:28 2016 -0400 @@ -36,7 +36,7 @@ class win32 { friend class os; - friend unsigned __stdcall java_start(class Thread*); + friend unsigned __stdcall thread_native_entry(class Thread*); protected: static int _vm_page_size; diff -r 96e35aced4ef -r 6b194cfc1557 hotspot/src/share/vm/runtime/thread.hpp --- a/hotspot/src/share/vm/runtime/thread.hpp Wed May 11 00:40:59 2016 +0000 +++ b/hotspot/src/share/vm/runtime/thread.hpp Wed May 11 01:02:28 2016 -0400 @@ -96,6 +96,7 @@ // - GangWorker // - GCTaskThread // - JavaThread +// - various subclasses eg CompilerThread, ServiceThread // - WatcherThread class Thread: public ThreadShadow { @@ -314,8 +315,7 @@ // Manage Thread::current() void initialize_thread_current(); - private: - void clear_thread_current(); // needed for detaching JNI threads + void clear_thread_current(); // TLS cleanup needed before threads terminate public: // thread entry point @@ -743,6 +743,11 @@ // Constructor WatcherThread(); + // No destruction allowed + ~WatcherThread() { + guarantee(false, "WatcherThread deletion must fix the race with VM termination"); + } + // Tester bool is_Watcher_thread() const { return true; } diff -r 96e35aced4ef -r 6b194cfc1557 hotspot/src/share/vm/runtime/vmThread.cpp --- a/hotspot/src/share/vm/runtime/vmThread.cpp Wed May 11 00:40:59 2016 +0000 +++ b/hotspot/src/share/vm/runtime/vmThread.cpp Wed May 11 01:02:28 2016 -0400 @@ -226,16 +226,12 @@ } } - VMThread::VMThread() : NamedThread() { set_name("VM Thread"); } void VMThread::destroy() { - if (_vm_thread != NULL) { - delete _vm_thread; - _vm_thread = NULL; // VM thread is gone - } + _vm_thread = NULL; // VM thread is gone } void VMThread::run() { @@ -308,9 +304,9 @@ _terminate_lock->notify(); } - // Deletion must be done synchronously by the JNI DestroyJavaVM thread - // so that the VMThread deletion completes before the main thread frees - // up the CodeHeap. + // We are now racing with the VM termination being carried out in + // another thread, so we don't "delete this". Numerous threads don't + // get deleted when the VM terminates } diff -r 96e35aced4ef -r 6b194cfc1557 hotspot/src/share/vm/runtime/vmThread.hpp --- a/hotspot/src/share/vm/runtime/vmThread.hpp Wed May 11 00:40:59 2016 +0000 +++ b/hotspot/src/share/vm/runtime/vmThread.hpp Wed May 11 01:02:28 2016 -0400 @@ -104,6 +104,12 @@ // Constructor VMThread(); + // No destruction allowed + ~VMThread() { + guarantee(false, "VMThread deletion must fix the race with VM termination"); + } + + // Tester bool is_VM_thread() const { return true; } bool is_GC_thread() const { return true; }