RFR: 8229516: Thread.isInterrupted() always returns false after thread termination

[serguei.spitsyn at oracle.com]

Hi David,

The fix looks good to me.
I did not pay much attention to the Graal related changes though.
The test coverage for Serviceability is complete.
Running java/lang/instrument tests is not necessary.

Thanks,
Serguei


On 10/29/19 00:42, David Holmes wrote:
> Bug: https://bugs.openjdk.java.net/browse/JDK-8229516
> CSR: https://bugs.openjdk.java.net/browse/JDK-8232676 (already approved)
> webrev: http://cr.openjdk.java.net/~dholmes/8229516/webrev/
>
> This cross-cuts core-libs, hotspot-runtime and the JIT compilers, but 
> only in small pieces each. There is also a small touch to 
> serviceability code.
>
> This change is "simply" moving the "interrupted" field out of the 
> osThread and into the java.lang.Thread so that it can be set 
> independently of whether the thread is alive (and to make things 
> easier for loom in the near future). It is very straightforward:
>
> - old scheme
>   - interrupted field is in osThread
>   - VM can read/write directly
>   - Java code calls into VM to read/write
>
> - new scheme
>   - interrupted field is in java.lang.Thread
>   - VM has to use javaCalls to read/write "directly"
>   - Java code can read/write directly
>
> No changes to any of the semantics regarding the actual interrupt 
> mechanism. Special thanks to Patricio for tracking down a bug I had 
> introduced in that regard!
>
> Special Note (Hi Roger!): on windows we still need to set/clear the 
> _interrupt_event used by the Process.waitFor logic. To facilitate 
> clearing via Thread.interrupted() I had to introduce a native method 
> that is a no-op except on Windows. This seemed the cheapest and least 
> intrusive means to achieve this.
>
> Other changes revolve around the fact we used to have an intrinsic for 
> Thread.isInterrupted and that is not needed any more. So we strip some 
> code out of C1/C2.
>
> The changes in the JVMCI/Graal code are a bit more far reaching as 
> entire classes disappear. I've cc'd Doug and Tom at Vladimir's request 
> so that they can comment on the JVMCI changes and whether I have gone 
> too far or not far enough. There are a bunch of tests for interruption 
> in JVMCI that could potentially be deleted if they are only intended 
> to test the JVMCI handling of interrupt:
>
> ./jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.jtt/src/org/graalvm/compiler/jtt/threads/Thread_isInterrupted*.java 
>
>
> Testing:
>  - Tiers 1-3 on all Oracle platforms
>  - Focused testing on Linux x64:
>     - Stress runs of JSR166TestCase
>     - Anything that seems to use interrupt():
>       - JDK
>         - java/lang/Thread
>         - java/util/concurrent
>         - jdk/internal/loader/InterruptedClassLoad.java
>         - javax/management
>         - java/nio/file/Files
>         - java/nio/channels
>         - java/net/Socket/Timeouts.java
>         - java/lang/Runtime/shutdown/
>         - java/lang/ProcessBuilder/Basic.java
>         - com/sun/jdi/
>       - Hotspot
>         - vmTestbase/nsk/monitoring/
>         - vmTestbase/nsk/jdwp
>         - vmTestbase/nsk/jdb/
>         - vmTestbase/nsk/jdi/
>         - vmTestbase/nsk/jvmti/
>         - runtime/Thread
>         - serviceability/jvmti/
>         - serviceability/jdwp
>         - serviceability/sa
>
> Thanks,
> David
> -----