RFR: 8335269: [Graal] occasional timeout in java/lang/StringBuffer/TestSynchronization.java with loom [v5]

[Doug Simon]

On Thu, 11 Jul 2024 20:11:50 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> Please review the following simple fix. A pinned virtual thread calling Thread.yield() in a loop might never poll for safepoints if the compiler relies on a poll in native method Continuation.doYield while optimizing. This is a special native method that doesn't always poll for safepoints, and in particular it doesn't if the virtual thread is pinned due to owning monitors. Currently this scenario can be reproduced with the Graal compiler.
>> 
>> I included a test which reproduces the issue with Graal (couldn't reproduce the issue with c2). The test times out without the fix and passes with it. I also run the patch through mach5 tiers1-3.
>> 
>> Thanks,
>> Patricio
>
> Patricio Chilano Mateo has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add vm.compMode != Xcomp

src/hotspot/share/runtime/continuationFreezeThaw.cpp line 1582:

> 1580:     freeze_result res = entry->is_pinned() ? freeze_pinned_cs : freeze_pinned_monitor;
> 1581:     log_develop_trace(continuations)("=== end of freeze (fail %d)", res);
> 1582:     // Avoid Thread.yield() loops without safepoint polls (see 8335269).

Is an explicit reference to a JBS issue id like this still recommended practice? After all, it will be a prefix in the merged commit message.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20016#discussion_r1674711298