RFR: 8230424: Use platform independent code for Thread.interrupt support

[Daniel D. Daugherty]

On 9/13/19 5:12 AM, David Holmes wrote:
> webrev: http://cr.openjdk.java.net/~dholmes/8230424/webrev/

src/hotspot/os/posix/os_posix.cpp
     The old _sleepEvent code had a check for a NULL _SleepEvent
     before unparking.
     The old _ParkEvent code had a check for a NULL _ParkEvent
     before unparking.

     Update: The code review invite covered this:

       > - minor cleanups to the logic of interrupt() as we can never
       >   have NULL events as we are always guaranteed to be operating
       >   on a live thread.

src/hotspot/os/solaris/os_solaris.cpp
     No comments.

src/hotspot/os/windows/osThread_windows.cpp
     No comments.

src/hotspot/os/windows/osThread_windows.hpp
     No comments.

src/hotspot/os/windows/os_windows.cpp
     Where did the JSR166 support move to?
     Update: moved to thread.cpp so okay.

src/hotspot/share/jvmci/jvmciRuntime.cpp
     No comments.

src/hotspot/share/prims/jvm.cpp
     No comments.

src/hotspot/share/prims/jvmtiEnv.cpp
     No comments.

src/hotspot/share/prims/jvmtiRawMonitor.cpp
     No comments.

src/hotspot/share/runtime/objectMonitor.cpp
     No comments (the Self -> jt changes threw me for a second :-)).

src/hotspot/share/runtime/os.hpp
     No comments.

src/hotspot/share/runtime/osThread.cpp
     No comments.

src/hotspot/share/runtime/osThread.hpp
     No comments.

src/hotspot/share/runtime/thread.cpp
     No comments.

src/hotspot/share/runtime/thread.hpp
     No comments.


Thumbs up! Robbin flagged the one nit indent issue that I spotted.
I don't need to see another webrev.

Dan


> bug: https://bugs.openjdk.java.net/browse/JDK-8230424
>
> The next instalment of the interruption logic refactoring.
>
> Summary of changes:
> - the windows-specific actions in os:interrupt and os::is_interrupted 
> are pushed down into the windows-specific osThread code. This all 
> relates to the only-used-by-the-JDK _interrupt_event.
> - the POSIX variant of os::interrupt and os::is_interrupted is inlined 
> as platform-independent code in the Thread versions and those versions 
> are themselves moved to JavaThread as instance methods.
> - minor cleanups to the logic of interrupt() as we can never have NULL 
> events as we are always guaranteed to be operating on a live thread.
> - All call sites adjusted as needed.
>
> One minor wart in this refactor is that on Windows we will execute an 
> OrderAccess::release() after setting the interrupt state in the 
> platform code, and then execute an additional fence() in the shared 
> code. I don't think this is worth trying to "fix" - and hopefully if 
> we can get rid of the Windows _interrupt_event this will all go away.
>
> If you are wondering why Windows used OrderAccess::release while POSIX 
> used OrderAccess::fence, I have no idea, and I do not intend to change 
> either of them. The only safe change would be to use fence() in both 
> cases, which just makes the Windows wart worse - and unnecessarily so 
> as we have seen no bugs using the existing release().
>
> Testing:
>  - hotspot runtime
>  - Tiers 1-3
>
> Thanks,
> David