RFR(M): 8239024: Kitchensink24HStress.java failed due to timeout

[Robbin Ehn]

Hi Markus, thanks for fixing so quickly.

Two things which can be improved for us not as familiar as you with this code:

#1

   77   // The system can proceed to a safepoint
   78   // because even if the thread is a JavaThread,
   79   // it is running as _thread_in_native here.
   80   void wait() {

Can we assert this ?

#2

When looking at this piece of code, it's not obvious that wait() also acquires.
Maybe there is better name for the method?

   98     if (acquire(_thread)) {
   99       assert(is_owner(), "invariant");
  100       return;
  101     }
  102     wait();
  103   }

No need for another version, seems fine, thanks!
If it helps I'm fine if you skip 24h grace period and push before fork.

/Robbin

On 2020-06-10 19:11, Markus Gronlund wrote:
> Greetings,
> 
> Please review this fix to resolve a deadlock involving the JfrStream_lock.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8239024
> Webrev: http://cr.openjdk.java.net/~mgronlun/8239024/webrev00/
> Testing: jdk_jfr
> 
> This webrev is a diff against https://mail.openjdk.java.net/pipermail/hotspot-jfr-dev/2020-June/001513.html
> 
> Please see the full problem description in the bug.
> 
> Short comment:
> The JfrRotationLock is only used by a single thread (the JFR Recorder Thread) during normal operations. Its only purpose is to provide a mutex for a potential JFR Emergency dump thread corrupting in-flight serializations as part of crash dumps. Previously, this lock provided a slow path to have a JavaThread wait on the JfrMsg_lock (so that safepoints could proceed). But with the changes associated with JDK-8245113, these threads are now in state _thread_in_native, so this wait is replaced by a simple os::naked_short_sleep().
> 
> Thanks to Robbin Ehn for problem analysis.
> 
> Thanks
> Markus
>