8230424: Use platform independent code for Thread.interrupt support
8231094: os::sleep in assert message should be changed to JavaThread::sleep
Reviewed-by: rehn, dcubed
--- a/src/hotspot/os/posix/os_posix.cpp Tue Sep 17 09:20:59 2019 -0700
+++ b/src/hotspot/os/posix/os_posix.cpp Tue Sep 17 19:09:37 2019 -0400
@@ -640,61 +640,6 @@
return;
}
-////////////////////////////////////////////////////////////////////////////////
-// interrupt support
-
-void os::interrupt(Thread* thread) {
- debug_only(Thread::check_for_dangling_thread_pointer(thread);)
- assert(thread->is_Java_thread(), "invariant");
- JavaThread* jt = (JavaThread*) thread;
- OSThread* osthread = thread->osthread();
-
- if (!osthread->interrupted()) {
- osthread->set_interrupted(true);
- // More than one thread can get here with the same value of osthread,
- // resulting in multiple notifications. We do, however, want the store
- // to interrupted() to be visible to other threads before we execute unpark().
- OrderAccess::fence();
- ParkEvent * const slp = jt->_SleepEvent ;
- if (slp != NULL) slp->unpark() ;
- }
-
- // For JSR166. Unpark even if interrupt status already was set
- jt->parker()->unpark();
-
- ParkEvent * ev = thread->_ParkEvent ;
- if (ev != NULL) ev->unpark() ;
-}
-
-bool os::is_interrupted(Thread* thread, bool clear_interrupted) {
- debug_only(Thread::check_for_dangling_thread_pointer(thread);)
-
- OSThread* osthread = thread->osthread();
-
- bool interrupted = osthread->interrupted();
-
- // NOTE that since there is no "lock" around the interrupt and
- // is_interrupted operations, there is the possibility that the
- // interrupted flag (in osThread) will be "false" but that the
- // low-level events will be in the signaled state. This is
- // intentional. The effect of this is that Object.wait() and
- // LockSupport.park() will appear to have a spurious wakeup, which
- // is allowed and not harmful, and the possibility is so rare that
- // it is not worth the added complexity to add yet another lock.
- // For the sleep event an explicit reset is performed on entry
- // to JavaThread::sleep, so there is no early return. It has also been
- // recommended not to put the interrupted flag into the "event"
- // structure because it hides the issue.
- if (interrupted && clear_interrupted) {
- osthread->set_interrupted(false);
- // consider thread->_SleepEvent->reset() ... optional optimization
- }
-
- return interrupted;
-}
-
-
-
static const struct {
int sig; const char* name;
}
@@ -2107,7 +2052,7 @@
// Optional optimization -- avoid state transitions if there's
// an interrupt pending.
- if (Thread::is_interrupted(thread, false)) {
+ if (jt->is_interrupted(false)) {
return;
}
@@ -2130,7 +2075,7 @@
// Don't wait if cannot get lock since interference arises from
// unparking. Also re-check interrupt before trying wait.
- if (Thread::is_interrupted(thread, false) ||
+ if (jt->is_interrupted(false) ||
pthread_mutex_trylock(_mutex) != 0) {
return;
}
--- a/src/hotspot/os/solaris/os_solaris.cpp Tue Sep 17 09:20:59 2019 -0700
+++ b/src/hotspot/os/solaris/os_solaris.cpp Tue Sep 17 19:09:37 2019 -0400
@@ -5063,7 +5063,7 @@
Thread* thread = Thread::current();
assert(thread->is_Java_thread(), "Must be JavaThread");
JavaThread *jt = (JavaThread *)thread;
- if (Thread::is_interrupted(thread, false)) {
+ if (jt->is_interrupted(false)) {
return;
}
@@ -5088,7 +5088,7 @@
// Don't wait if cannot get lock since interference arises from
// unblocking. Also. check interrupt before trying wait
- if (Thread::is_interrupted(thread, false) ||
+ if (jt->is_interrupted(false) ||
os::Solaris::mutex_trylock(_mutex) != 0) {
return;
}
--- a/src/hotspot/os/windows/osThread_windows.cpp Tue Sep 17 09:20:59 2019 -0700
+++ b/src/hotspot/os/windows/osThread_windows.cpp Tue Sep 17 19:09:37 2019 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1998, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2019, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -23,12 +23,9 @@
*/
// no precompiled headers
-#include "runtime/handles.inline.hpp"
-#include "runtime/mutexLocker.hpp"
+#include "runtime/orderAccess.hpp"
#include "runtime/os.hpp"
#include "runtime/osThread.hpp"
-#include "runtime/safepoint.hpp"
-#include "runtime/vmThread.hpp"
void OSThread::pd_initialize() {
set_thread_handle(NULL);
@@ -36,8 +33,34 @@
set_interrupt_event(NULL);
}
-// TODO: this is not well encapsulated; creation and deletion of the
-// interrupt_event are done in os_win32.cpp, create_thread and
-// free_thread. Should follow pattern of Linux/Solaris code here.
void OSThread::pd_destroy() {
+ if (_interrupt_event != NULL) {
+ CloseHandle(_interrupt_event);
+ }
}
+
+// We need to specialize these to interact with the _interrupt_event.
+
+volatile bool OSThread::interrupted() {
+ return _interrupted != 0 &&
+ (WaitForSingleObject(_interrupt_event, 0) == WAIT_OBJECT_0);
+}
+
+void OSThread::set_interrupted(bool z) {
+ if (z) {
+ _interrupted = 1;
+ // More than one thread can get here with the same value of osthread,
+ // resulting in multiple notifications. We do, however, want the store
+ // to interrupted() to be visible to other threads before we post
+ // the interrupt event.
+ OrderAccess::release();
+ SetEvent(_interrupt_event);
+ }
+ else {
+ // We should only ever clear the interrupt if we are in fact interrupted,
+ // and this can only be done by the current thread on itself.
+ assert(_interrupted == 1, "invariant for clearing interrupt state");
+ _interrupted = 0;
+ ResetEvent(_interrupt_event);
+ }
+}
--- a/src/hotspot/os/windows/osThread_windows.hpp Tue Sep 17 09:20:59 2019 -0700
+++ b/src/hotspot/os/windows/osThread_windows.hpp Tue Sep 17 19:09:37 2019 -0400
@@ -32,7 +32,8 @@
private:
// Win32-specific thread information
HANDLE _thread_handle; // Win32 thread handle
- HANDLE _interrupt_event; // Event signalled on thread interrupt
+ HANDLE _interrupt_event; // Event signalled on thread interrupt for use by
+ // Process.waitFor().
ThreadState _last_state;
public:
@@ -42,6 +43,11 @@
void set_thread_handle(HANDLE handle) { _thread_handle = handle; }
HANDLE interrupt_event() const { return _interrupt_event; }
void set_interrupt_event(HANDLE interrupt_event) { _interrupt_event = interrupt_event; }
+ // These are specialized on Windows to interact with the _interrupt_event.
+ // Also note that Windows does not skip these calls if we are interrupted - see
+ // LibraryCallKit::inline_native_isInterrupted
+ volatile bool interrupted();
+ void set_interrupted(bool z);
#ifndef PRODUCT
// Used for debugging, return a unique integer for each thread.
@@ -54,7 +60,6 @@
return false;
}
#endif // ASSERT
- bool is_try_mutex_enter() { return false; }
// This is a temporary fix for the thread states during
// suspend/resume until we throw away OSThread completely.
--- a/src/hotspot/os/windows/os_windows.cpp Tue Sep 17 09:20:59 2019 -0700
+++ b/src/hotspot/os/windows/os_windows.cpp Tue Sep 17 19:09:37 2019 -0400
@@ -612,7 +612,9 @@
return false;
}
osthread->set_interrupt_event(interrupt_event);
- osthread->set_interrupted(false);
+ // We don't call set_interrupted(false) as it will trip the assert in there
+ // as we are not operating on the current thread. We don't need to call it
+ // because the initial state is already correct.
thread->set_osthread(osthread);
@@ -684,7 +686,6 @@
if (thread_handle == NULL) {
// Need to clean up stuff we've allocated so far
- CloseHandle(osthread->interrupt_event());
thread->set_osthread(NULL);
delete osthread;
return false;
@@ -714,7 +715,6 @@
"os::free_thread but not current thread");
CloseHandle(osthread->thread_handle());
- CloseHandle(osthread->interrupt_event());
delete osthread;
}
@@ -3485,7 +3485,6 @@
}
-
// Short sleep, direct OS call.
//
// ms = 0, means allow others (if any) to run.
@@ -3593,49 +3592,6 @@
return OS_OK;
}
-void os::interrupt(Thread* thread) {
- debug_only(Thread::check_for_dangling_thread_pointer(thread);)
- assert(thread->is_Java_thread(), "invariant");
- JavaThread* jt = (JavaThread*) thread;
- OSThread* osthread = thread->osthread();
- osthread->set_interrupted(true);
- // More than one thread can get here with the same value of osthread,
- // resulting in multiple notifications. We do, however, want the store
- // to interrupted() to be visible to other threads before we post
- // the interrupt event.
- OrderAccess::release();
- SetEvent(osthread->interrupt_event());
- // For JSR166: unpark after setting status
- jt->parker()->unpark();
-
- ParkEvent * ev = thread->_ParkEvent;
- if (ev != NULL) ev->unpark();
-
- ev = jt->_SleepEvent;
- if (ev != NULL) ev->unpark();
-}
-
-
-bool os::is_interrupted(Thread* thread, bool clear_interrupted) {
- debug_only(Thread::check_for_dangling_thread_pointer(thread);)
-
- OSThread* osthread = thread->osthread();
- // There is no synchronization between the setting of the interrupt
- // and it being cleared here. It is critical - see 6535709 - that
- // we only clear the interrupt state, and reset the interrupt event,
- // if we are going to report that we were indeed interrupted - else
- // an interrupt can be "lost", leading to spurious wakeups or lost wakeups
- // depending on the timing. By checking thread interrupt event to see
- // if the thread gets real interrupt thus prevent spurious wakeup.
- bool interrupted = osthread->interrupted() && (WaitForSingleObject(osthread->interrupt_event(), 0) == WAIT_OBJECT_0);
- if (interrupted && clear_interrupted) {
- osthread->set_interrupted(false);
- ResetEvent(osthread->interrupt_event());
- } // Otherwise leave the interrupted state alone
-
- return interrupted;
-}
-
// GetCurrentThreadId() returns DWORD
intx os::current_thread_id() { return GetCurrentThreadId(); }
@@ -5346,7 +5302,7 @@
JavaThread* thread = JavaThread::current();
// Don't wait if interrupted or already triggered
- if (Thread::is_interrupted(thread, false) ||
+ if (thread->is_interrupted(false) ||
WaitForSingleObject(_ParkEvent, 0) == WAIT_OBJECT_0) {
ResetEvent(_ParkEvent);
return;
--- a/src/hotspot/share/jvmci/jvmciRuntime.cpp Tue Sep 17 09:20:59 2019 -0700
+++ b/src/hotspot/share/jvmci/jvmciRuntime.cpp Tue Sep 17 19:09:37 2019 -0400
@@ -631,7 +631,7 @@
// The other thread may exit during this process, which is ok so return false.
return JNI_FALSE;
} else {
- return (jint) Thread::is_interrupted(receiverThread, clear_interrupted != 0);
+ return (jint) receiverThread->is_interrupted(clear_interrupted != 0);
}
JRT_END
--- a/src/hotspot/share/prims/jvm.cpp Tue Sep 17 09:20:59 2019 -0700
+++ b/src/hotspot/share/prims/jvm.cpp Tue Sep 17 19:09:37 2019 -0400
@@ -2974,7 +2974,7 @@
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), "timeout value is negative");
}
- if (Thread::is_interrupted (THREAD, true) && !HAS_PENDING_EXCEPTION) {
+ if (thread->is_interrupted(true) && !HAS_PENDING_EXCEPTION) {
THROW_MSG(vmSymbols::java_lang_InterruptedException(), "sleep interrupted");
}
@@ -3071,7 +3071,7 @@
bool is_alive = tlh.cv_internal_thread_to_JavaThread(jthread, &receiver, NULL);
if (is_alive) {
// jthread refers to a live JavaThread.
- Thread::interrupt(receiver);
+ receiver->interrupt();
}
JVM_END
@@ -3084,7 +3084,7 @@
bool is_alive = tlh.cv_internal_thread_to_JavaThread(jthread, &receiver, NULL);
if (is_alive) {
// jthread refers to a live JavaThread.
- return (jboolean) Thread::is_interrupted(receiver, clear_interrupted != 0);
+ return (jboolean) receiver->is_interrupted(clear_interrupted != 0);
} else {
return JNI_FALSE;
}
--- a/src/hotspot/share/prims/jvmtiEnv.cpp Tue Sep 17 09:20:59 2019 -0700
+++ b/src/hotspot/share/prims/jvmtiEnv.cpp Tue Sep 17 19:09:37 2019 -0400
@@ -1101,7 +1101,7 @@
return err;
}
- Thread::interrupt(java_thread);
+ java_thread->interrupt();
return JVMTI_ERROR_NONE;
} /* end InterruptThread */
--- a/src/hotspot/share/prims/jvmtiRawMonitor.cpp Tue Sep 17 09:20:59 2019 -0700
+++ b/src/hotspot/share/prims/jvmtiRawMonitor.cpp Tue Sep 17 19:09:37 2019 -0400
@@ -373,8 +373,12 @@
OrderAccess::fence() ;
// check interrupt event
- if (interruptible && Thread::is_interrupted(THREAD, true)) {
- return OM_INTERRUPTED;
+ if (interruptible) {
+ assert(THREAD->is_Java_thread(), "Only JavaThreads can be interruptible");
+ JavaThread* jt = (JavaThread*) THREAD;
+ if (jt->is_interrupted(true)) {
+ return OM_INTERRUPTED;
+ }
}
intptr_t save = _recursions ;
@@ -401,8 +405,11 @@
}
guarantee (THREAD == _owner, "invariant") ;
- if (interruptible && Thread::is_interrupted(THREAD, true)) {
- return OM_INTERRUPTED;
+ if (interruptible) {
+ JavaThread* jt = (JavaThread*) THREAD;
+ if (jt->is_interrupted(true)) {
+ return OM_INTERRUPTED;
+ }
}
return OM_OK ;
}
--- a/src/hotspot/share/runtime/objectMonitor.cpp Tue Sep 17 09:20:59 2019 -0700
+++ b/src/hotspot/share/runtime/objectMonitor.cpp Tue Sep 17 19:09:37 2019 -0400
@@ -1205,7 +1205,7 @@
EventJavaMonitorWait event;
// check for a pending interrupt
- if (interruptible && Thread::is_interrupted(Self, true) && !HAS_PENDING_EXCEPTION) {
+ if (interruptible && jt->is_interrupted(true) && !HAS_PENDING_EXCEPTION) {
// post monitor waited event. Note that this is past-tense, we are done waiting.
if (JvmtiExport::should_post_monitor_waited()) {
// Note: 'false' parameter is passed here because the
@@ -1275,7 +1275,7 @@
// Thread is in thread_blocked state and oop access is unsafe.
jt->set_suspend_equivalent();
- if (interruptible && (Thread::is_interrupted(THREAD, false) || HAS_PENDING_EXCEPTION)) {
+ if (interruptible && (jt->is_interrupted(false) || HAS_PENDING_EXCEPTION)) {
// Intentionally empty
} else if (node._notified == 0) {
if (millis <= 0) {
@@ -1401,7 +1401,7 @@
if (!WasNotified) {
// no, it could be timeout or Thread.interrupt() or both
// check for interrupt event, otherwise it is timeout
- if (interruptible && Thread::is_interrupted(Self, true) && !HAS_PENDING_EXCEPTION) {
+ if (interruptible && jt->is_interrupted(true) && !HAS_PENDING_EXCEPTION) {
THROW(vmSymbols::java_lang_InterruptedException());
}
}
--- a/src/hotspot/share/runtime/os.hpp Tue Sep 17 09:20:59 2019 -0700
+++ b/src/hotspot/share/runtime/os.hpp Tue Sep 17 19:09:37 2019 -0400
@@ -480,9 +480,6 @@
static OSReturn set_priority(Thread* thread, ThreadPriority priority);
static OSReturn get_priority(const Thread* const thread, ThreadPriority& priority);
- static void interrupt(Thread* thread);
- static bool is_interrupted(Thread* thread, bool clear_interrupted);
-
static int pd_self_suspend_thread(Thread* thread);
static ExtendedPC fetch_frame_from_context(const void* ucVoid, intptr_t** sp, intptr_t** fp);
--- a/src/hotspot/share/runtime/osThread.cpp Tue Sep 17 09:20:59 2019 -0700
+++ b/src/hotspot/share/runtime/osThread.cpp Tue Sep 17 19:09:37 2019 -0400
@@ -30,7 +30,7 @@
pd_initialize();
set_start_proc(start_proc);
set_start_parm(start_parm);
- set_interrupted(false);
+ _interrupted = 0;
}
OSThread::~OSThread() {
--- a/src/hotspot/share/runtime/osThread.hpp Tue Sep 17 09:20:59 2019 -0700
+++ b/src/hotspot/share/runtime/osThread.hpp Tue Sep 17 19:09:37 2019 -0400
@@ -82,10 +82,11 @@
void set_start_proc(OSThreadStartFunc start_proc) { _start_proc = start_proc; }
void* start_parm() const { return _start_parm; }
void set_start_parm(void* start_parm) { _start_parm = start_parm; }
-
+ // These are specialized on Windows.
+#ifndef _WINDOWS
volatile bool interrupted() const { return _interrupted != 0; }
void set_interrupted(bool z) { _interrupted = z ? 1 : 0; }
-
+#endif
// Printing
void print_on(outputStream* st) const;
void print() const;
--- a/src/hotspot/share/runtime/thread.cpp Tue Sep 17 09:20:59 2019 -0700
+++ b/src/hotspot/share/runtime/thread.cpp Tue Sep 17 19:09:37 2019 -0400
@@ -856,19 +856,6 @@
return true;
}
-void Thread::interrupt(Thread* thread) {
- debug_only(check_for_dangling_thread_pointer(thread);)
- os::interrupt(thread);
-}
-
-bool Thread::is_interrupted(Thread* thread, bool clear_interrupted) {
- debug_only(check_for_dangling_thread_pointer(thread);)
- // Note: If clear_interrupted==false, this simply fetches and
- // returns the value of the field osthread()->interrupted().
- return os::is_interrupted(thread, clear_interrupted);
-}
-
-
// GC Support
bool Thread::claim_par_threads_do(uintx claim_token) {
uintx token = _threads_do_token;
@@ -1726,6 +1713,56 @@
assert(deferred_card_mark().is_empty(), "Default MemRegion ctor");
}
+
+// interrupt support
+
+void JavaThread::interrupt() {
+ debug_only(check_for_dangling_thread_pointer(this);)
+
+ if (!osthread()->interrupted()) {
+ osthread()->set_interrupted(true);
+ // More than one thread can get here with the same value of osthread,
+ // resulting in multiple notifications. We do, however, want the store
+ // to interrupted() to be visible to other threads before we execute unpark().
+ OrderAccess::fence();
+
+ // For JavaThread::sleep. Historically we only unpark if changing to the interrupted
+ // state, in contrast to the other events below. Not clear exactly why.
+ _SleepEvent->unpark();
+ }
+
+ // For JSR166. Unpark even if interrupt status already was set.
+ parker()->unpark();
+
+ // For ObjectMonitor and JvmtiRawMonitor
+ _ParkEvent->unpark();
+}
+
+
+bool JavaThread::is_interrupted(bool clear_interrupted) {
+ debug_only(check_for_dangling_thread_pointer(this);)
+ bool interrupted = osthread()->interrupted();
+
+ // NOTE that since there is no "lock" around the interrupt and
+ // is_interrupted operations, there is the possibility that the
+ // interrupted flag (in osThread) will be "false" but that the
+ // low-level events will be in the signaled state. This is
+ // intentional. The effect of this is that Object.wait() and
+ // LockSupport.park() will appear to have a spurious wakeup, which
+ // is allowed and not harmful, and the possibility is so rare that
+ // it is not worth the added complexity to add yet another lock.
+ // For the sleep event an explicit reset is performed on entry
+ // to JavaThread::sleep, so there is no early return. It has also been
+ // recommended not to put the interrupted flag into the "event"
+ // structure because it hides the issue.
+ if (interrupted && clear_interrupted) {
+ osthread()->set_interrupted(false);
+ // consider thread->_SleepEvent->reset() ... optional optimization
+ }
+
+ return interrupted;
+}
+
bool JavaThread::reguard_stack(address cur_sp) {
if (_stack_guard_state != stack_guard_yellow_reserved_disabled
&& _stack_guard_state != stack_guard_reserved_disabled) {
@@ -2370,8 +2407,8 @@
}
- // Interrupt thread so it will wake up from a potential wait()
- Thread::interrupt(this);
+ // Interrupt thread so it will wake up from a potential wait()/sleep()/park()
+ this->interrupt();
}
// External suspension mechanism.
@@ -3361,7 +3398,7 @@
for (;;) {
// interruption has precedence over timing out
- if (os::is_interrupted(this, true)) {
+ if (this->is_interrupted(true)) {
return false;
}
@@ -3389,7 +3426,7 @@
// time moving backwards, should only happen if no monotonic clock
// not a guarantee() because JVM should not abort on kernel/glibc bugs
assert(!os::supports_monotonic_clock(),
- "unexpected time moving backwards detected in os::sleep()");
+ "unexpected time moving backwards detected in JavaThread::sleep()");
} else {
millis -= (newtime - prevtime) / NANOSECS_PER_MILLISEC;
}
--- a/src/hotspot/share/runtime/thread.hpp Tue Sep 17 09:20:59 2019 -0700
+++ b/src/hotspot/share/runtime/thread.hpp Tue Sep 17 19:09:37 2019 -0400
@@ -514,8 +514,6 @@
static void set_priority(Thread* thread, ThreadPriority priority);
static ThreadPriority get_priority(const Thread* const thread);
static void start(Thread* thread);
- static void interrupt(Thread* thr);
- static bool is_interrupted(Thread* thr, bool clear_interrupted);
void set_native_thread_name(const char *name) {
assert(Thread::current() == this, "set_native_thread_name can only be called on the current thread");
@@ -2055,9 +2053,14 @@
InstanceKlass* _class_to_be_initialized;
// java.lang.Thread.sleep support
+ ParkEvent * _SleepEvent;
public:
- ParkEvent * _SleepEvent;
bool sleep(jlong millis);
+
+ // java.lang.Thread interruption support
+ void interrupt();
+ bool is_interrupted(bool clear_interrupted);
+
};
// Inline implementation of JavaThread::current