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

[David Holmes]

Thanks Doug! Appreciate the help getting the Graal bits correct.

David

On 31/10/2019 5:47 pm, Doug Simon wrote:
> 
> 
>> On 31 Oct 2019, at 05:30, David Holmes <david.holmes at oracle.com 
>> <mailto: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/
> 
> Looks good.
> 
> -Doug