RFR 8227528: TestAbortVMOnSafepointTimeout.java failed due to "RuntimeException: 'Safepoint sync time longer than' missing from stdout/stderr"

[Daniel D. Daugherty]

On 7/26/19 2:46 PM, Patricio Chilano wrote:
> Hi all,
>
> Could you review this small fix for test 
> TestAbortVMOnSafepointTimeout.java?
>
> The test has been failing intermittently since 8191890. As explained 
> in the bug comments, it turns out that a bias revocation handshake 
> could happen in between the start of the "for" loop without safepoint 
> polls and the safepoint where we want to timeout. That allows for the 
> long loop to actually finish and prevents the desired timeout in the 
> later safepoint. The simple solution is to just avoid using biased 
> locking in this test (and therefore prevent the revocation handshake), 
> since we just want to test the correct behavior of flag 
> AbortVMOnSafepointTimeout.
>
> Webrev: http://cr.openjdk.java.net/~pchilanomate/8227528/v01/webrev

The change itself is trivial. However, the reasons behind the change aren't.

This part of the description caught my eye:

     the start of the "for" loop without safepoint polls

and my brain did a "Say what?!?!" Of course, that was without looking at
the test which has a huge number of options, including these:

     L70:                 "-XX:-UseCountedLoopSafepoints",
     L71:                 "-XX:LoopStripMiningIter=0",
     L72:                 "-XX:LoopUnrollLimit=0",

Okay, now the world makes much more sense. We are intentionally telling
the compiler to not emit safepoint polls in the counted loop and we're
turning off other loop optimizations. Basically, we're telling the
compiler we want to stall in that loop until we exceed the safepoint
timeout limit. Got it...

So the new biased locking handshake messes with the timeout that this
test is trying to achieve. Disabling biased locking makes the test more
robust by allowing the safepoint sync timeout to happen.

A couple of minor suggestions:

test/hotspot/jtreg/runtime/Safepoint/TestAbortVMOnSafepointTimeout.java
     L30:  * @bug 8219584

         You should add an @bug for this bug (8227528). I don't know if
         you can put more than one bug ID on an @bug line or if you need
         a separate @bug line.

     L61:         ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
         Please add a comment above this line:

             // -XX:-UseBiasedLocking - is used to prevent biased locking
             // handshakes from changing the timing of this test.

Thumbs up. I don't need to see another webrev if you choose to make
the above changes.

Dan


> Bugid: https://bugs.openjdk.java.net/browse/JDK-8227528
>
> Thanks!
> Patricio