Re: 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.hol...@oracle.com <mailto:david.hol...@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.hol...@oracle.com <mailto:david.hol...@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