[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8229516: Thread.isInterrupted() always returns false after thread termination
From:       David Holmes <david.holmes () oracle ! com>
Date:       2019-10-31 8:45:00
Message-ID: 57b1a2ad-2361-dfee-0085-2098fefe1d66 () oracle ! com
[Download RAW message or body]

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


[prev in list] [next in list] [prev in thread] [next in thread]