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

src/hotspot/os/posix/os_posix.cpp

@@ -640,61 +640,6 @@ void os::naked_short_sleep(jlong ms) {
   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 @@ void Parker::park(bool isAbsolute, jlong time) {
 
   // 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 @@ void Parker::park(bool isAbsolute, jlong time) {
 
   // 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;
   }

src/hotspot/os/solaris/os_solaris.cpp

@@ -5063,7 +5063,7 @@ void Parker::park(bool isAbsolute, jlong time) {
   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 @@ void Parker::park(bool isAbsolute, jlong time) {
 
   // 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;
   }

src/hotspot/os/windows/osThread_windows.cpp

@@ -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/orderAccess.hpp"
-#include "runtime/mutexLocker.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 @@ void OSThread::pd_initialize() {
   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);
+  }
 }

src/hotspot/os/windows/osThread_windows.hpp

@@ -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.

src/hotspot/os/windows/os_windows.cpp

@@ -612,7 +612,9 @@ bool os::create_thread(Thread* thread, ThreadType thr_type,
     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 @@ bool os::create_thread(Thread* thread, ThreadType thr_type,
 
   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 @@ void os::free_thread(OSThread* osthread) {
          "os::free_thread but not current thread");
 
   CloseHandle(osthread->thread_handle());
-  CloseHandle(osthread->interrupt_event());
   delete osthread;
 }
 
@@ -3485,7 +3485,6 @@ void os::pd_start_thread(Thread* thread) {
 }
 
 
-
 // Short sleep, direct OS call.
 //
 // ms = 0, means allow others (if any) to run.
@@ -3593,49 +3592,6 @@ OSReturn os::get_native_priority(const Thread* const thread,
   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 @@ void Parker::park(bool isAbsolute, jlong time) {
   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;

src/hotspot/share/jvmci/jvmciRuntime.cpp

@@ -631,7 +631,7 @@ JRT_ENTRY(jboolean, JVMCIRuntime::thread_is_interrupted(JavaThread* thread, oopD
     // 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
 

src/hotspot/share/prims/jvm.cpp

@@ -2974,7 +2974,7 @@ JVM_ENTRY(void, JVM_Sleep(JNIEnv* env, jclass threadClass, jlong millis))
     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 @@ JVM_ENTRY(void, JVM_Interrupt(JNIEnv* env, jobject jthread))
   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 @@ JVM_ENTRY(jboolean, JVM_IsInterrupted(JNIEnv* env, jobject jthread, jboolean cle
   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;
   }

src/hotspot/share/prims/jvmtiEnv.cpp

@@ -1101,7 +1101,7 @@ JvmtiEnv::InterruptThread(jthread thread) {
     return err;
   }
 
-  Thread::interrupt(java_thread);
+  java_thread->interrupt();
 
   return JVMTI_ERROR_NONE;
 } /* end InterruptThread */

src/hotspot/share/prims/jvmtiRawMonitor.cpp

@@ -373,8 +373,12 @@ int JvmtiRawMonitor::raw_wait(jlong millis, bool interruptible, TRAPS) {
   OrderAccess::fence() ;
 
   // check interrupt event
-  if (interruptible && Thread::is_interrupted(THREAD, true)) {
+  if (interruptible) {
-    return OM_INTERRUPTED;
+    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 @@ int JvmtiRawMonitor::raw_wait(jlong millis, bool interruptible, TRAPS) {
   }
   guarantee (THREAD == _owner, "invariant") ;
 
-  if (interruptible && Thread::is_interrupted(THREAD, true)) {
+  if (interruptible) {
-    return OM_INTERRUPTED;
+    JavaThread* jt = (JavaThread*) THREAD;
+    if (jt->is_interrupted(true)) {
+      return OM_INTERRUPTED;
+    }
   }
   return OM_OK ;
 }

src/hotspot/share/runtime/objectMonitor.cpp

@@ -1205,7 +1205,7 @@ void ObjectMonitor::wait(jlong millis, bool interruptible, TRAPS) {
   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 @@ void ObjectMonitor::wait(jlong millis, bool interruptible, TRAPS) {
       // 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 @@ void ObjectMonitor::wait(jlong millis, bool interruptible, TRAPS) {
   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());
     }
   }

src/hotspot/share/runtime/os.hpp

@@ -480,9 +480,6 @@ class os: AllStatic {
   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);

src/hotspot/share/runtime/osThread.cpp

@@ -30,7 +30,7 @@ OSThread::OSThread(OSThreadStartFunc start_proc, void* start_parm) {
   pd_initialize();
   set_start_proc(start_proc);
   set_start_parm(start_parm);
-  set_interrupted(false);
+  _interrupted = 0;
 }
 
 OSThread::~OSThread() {

src/hotspot/share/runtime/osThread.hpp

@@ -82,10 +82,11 @@ class OSThread: public CHeapObj<mtThread> {
   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;

src/hotspot/share/runtime/thread.cpp

@@ -856,19 +856,6 @@ JavaThread::is_thread_fully_suspended(bool wait_for_suspend, uint32_t *bits) {
   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 @@ JavaThread::JavaThread(bool is_attaching_via_jni) :
   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 @@ void JavaThread::send_thread_stop(oop java_throwable)  {
   }
 
 
-  // Interrupt thread so it will wake up from a potential wait()
+  // Interrupt thread so it will wake up from a potential wait()/sleep()/park()
-  Thread::interrupt(this);
+  this->interrupt();
 }
 
 // External suspension mechanism.
@@ -3361,7 +3398,7 @@ bool JavaThread::sleep(jlong millis) {
 
   for (;;) {
     // interruption has precedence over timing out
-    if (os::is_interrupted(this, true)) {
+    if (this->is_interrupted(true)) {
       return false;
     }
 
@@ -3389,7 +3426,7 @@ bool JavaThread::sleep(jlong millis) {
       // 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;
     }

src/hotspot/share/runtime/thread.hpp

@@ -514,8 +514,6 @@ class Thread: public ThreadShadow {
   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 @@ private:
   InstanceKlass* _class_to_be_initialized;
 
   // java.lang.Thread.sleep support
-public:
   ParkEvent * _SleepEvent;
+public:
   bool sleep(jlong millis);
+
+  // java.lang.Thread interruption support
+  void interrupt();
+  bool is_interrupted(bool clear_interrupted);
+
 };
 
 // Inline implementation of JavaThread::current