8154715: Missing destructor and/or TLS clearing calls for terminating threads

Clear TLS on return from thread->run() unless already done. Prohibit asynchronous thread deletion.

Reviewed-by: stefank, sspitsyn
This commit is contained in:
Brian Gardner 2016-05-11 01:02:28 -04:00 committed by David Holmes
parent cdc0b9792f
commit 0e8376bd17
9 changed files with 116 additions and 52 deletions

View File

@ -777,7 +777,7 @@ bool os::Aix::get_meminfo(meminfo_t* pmi) {
// 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 @@ static void *java_start(Thread *thread) {
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 @@ bool os::create_thread(Thread* thread, ThreadType thr_type, size_t stack_size) {
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::pd_start_thread(Thread* thread) {
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;
}

View File

@ -665,7 +665,7 @@ static uint64_t locate_unique_thread_id(mach_port_t mach_thread_port) {
#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 @@ static void *java_start(Thread *thread) {
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 @@ bool os::create_thread(Thread* thread, ThreadType thr_type, size_t 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];
if (ret == 0) {
@ -889,11 +898,14 @@ void os::pd_start_thread(Thread* thread) {
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;
}

View File

@ -638,7 +638,7 @@ bool os::Linux::manually_expand_stack(JavaThread * t, address addr) {
// 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 @@ static void *java_start(Thread *thread) {
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 @@ bool os::create_thread(Thread* thread, ThreadType thr_type,
{
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::pd_start_thread(Thread* thread) {
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(&current);
pthread_sigmask(SIG_SETMASK, NULL, &current);
assert(!sigismember(&current, SR_signum), "SR signal should not be blocked!");
sigset_t current;
sigemptyset(&current);
pthread_sigmask(SIG_SETMASK, NULL, &current);
assert(!sigismember(&current, 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;
}

View File

@ -725,8 +725,8 @@ extern "C" void breakpoint() {
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 @@ extern "C" void* java_start(void* thread_addr) {
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 @@ bool os::create_thread(Thread* thread, ThreadType thr_type,
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::initialize_thread(Thread* thr) {
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;
}

View File

@ -409,8 +409,8 @@ struct tm* os::localtime_pd(const time_t* clock, struct tm* res) {
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 @@ static unsigned __stdcall java_start(Thread* thread) {
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 @@ bool os::create_thread(Thread* thread, ThreadType thr_type,
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 @@ bool os::create_thread(Thread* thread, ThreadType thr_type,
// 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;

View File

@ -36,7 +36,7 @@ static const char* path_separator() { return ";"; }
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;

View File

@ -96,6 +96,7 @@ class WorkerThread;
// - GangWorker
// - GCTaskThread
// - JavaThread
// - various subclasses eg CompilerThread, ServiceThread
// - WatcherThread
class Thread: public ThreadShadow {
@ -314,8 +315,7 @@ class Thread: public ThreadShadow {
// 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 @@ class WatcherThread: public Thread {
// 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; }

View File

@ -226,16 +226,12 @@ void VMThread::create() {
}
}
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 @@ void VMThread::run() {
_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
}

View File

@ -104,6 +104,12 @@ class VMThread: public NamedThread {
// 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; }