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

[Doug Simon]

> On 31 Oct 2019, at 05:30, David Holmes <david.holmes at oracle.com> wrote:
> 
> Hi Doug,
> 
> On 30/10/2019 10:06 pm, Doug Simon wrote:
>>> On 30 Oct 2019, at 05:28, David Holmes <david.holmes at oracle.com <mailto:david.holmes at oracle.com>> wrote:
>>> 
>>> Hi Doug,
>>> 
>>> Your proposed patch wasn't quite right. I made some adjustments but I'm still having issues with the test, HotSpotMethodSubstitutionTest.testThreadSubstitutions, as I don't see how to make the test execution conditional on the VM config.
>> Like this:
> 
> Thanks for that! One alteration below ...
> 
>> diff --git a/compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotMethodSubstitutionTest.java b/compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotMethodSubstitutionTest.java
>> index 96e7d979d36..3c928b86961 100644
>> --- a/compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotMethodSubstitutionTest.java
>> +++ b/compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotMethodSubstitutionTest.java
>> @@ -28,12 +28,13 @@ import java.lang.invoke.ConstantCallSite;
>>  import java.lang.invoke.MethodHandles;
>>  import java.lang.invoke.MethodType;
>> -import org.graalvm.compiler.nodes.IfNode;
>> -import org.junit.Test;
>> -
>>  import org.graalvm.compiler.api.directives.GraalDirectives;
>>  import org.graalvm.compiler.api.replacements.MethodSubstitution;
>> +import org.graalvm.compiler.hotspot.GraalHotSpotVMConfig;
>> +import org.graalvm.compiler.hotspot.HotSpotBackend;
>> +import org.graalvm.compiler.nodes.IfNode;
>>  import org.graalvm.compiler.replacements.test.MethodSubstitutionTest;
>> +import org.junit.Test;
>>  /**
>>   * Tests HotSpot specific {@link MethodSubstitution}s.
>> @@ -133,13 +134,18 @@ public class HotSpotMethodSubstitutionTest extends MethodSubstitutionTest {
>>      @Test
>>      public void testThreadSubstitutions() {
>> +        GraalHotSpotVMConfig config = ((HotSpotBackend) getBackend()).getRuntime().getVMConfig();
>>          testGraph("currentThread");
>> -        assertInGraph(testGraph("threadIsInterrupted", "isInterrupted", true), IfNode.class);
>> -        assertInGraph(testGraph("threadInterrupted", "isInterrupted", true), IfNode.class);
>> +        if (config.osThreadOffset != Integer.MAX_VALUE) {
> 
> s/osThreadOffset/osThreadInterruptedOffset/

Good catch. Sorry about that.
> 
> This change removes the interrupted field from osThread but not osThread itself. Though as osThread is only used to then access the interrupted field, I could both the same. Here's updated webrev showing that:
> 
> http://cr.openjdk.java.net/~dholmes/8229516/webrev.v2/ <http://cr.openjdk.java.net/~dholmes/8229516/webrev.v2/>

Looks good.

-Doug
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20191031/fd96b590/attachment.html>