RFR: 8314225: SIGSEGV in JavaThread::is_lock_owned

[Kevin Walls]

On Wed, 24 Apr 2024 19:50:08 GMT, Kevin Walls <kevinw at openjdk.org> wrote:

> Removal of JavaThread's MonitorChunks member.  This held lock information during deoptimization, but access to it is unnecessary for anything other than the deoptimization itself.
> 
> Access to it in is_lock_owned() was racy, and caused rare crashes.

JavaThread's MonitorChunks member is obsolete.

In lightweight locking, where an object has its mark word copied/displaced into a thread stack, owner checks can be made by checking if such a pointer is within the stack of a thread.

Lock inflation makes a lightweight lock into a heavyweight lock, and we always inflate during OSR and deoptimization, therefore monitor_chunks is obsolete.

BasicObjectLock::move_to(oop obj, BasicLock* dest) is called during deoptimization to move the BasicLocks to these chunks, and always inflates the monitor. It doesn’t change the object’s markword to point to this new address.

(Thanks to David, Dean and Patricio for talking this through!)


So:
JavaThread::is_lock_owned should not check and traverse the _monitor_chunks list.

src/hotspot/share/runtime/vframeArray.cpp:
This does not need to save MonitorChunks in the JavaThread, which means JavaThread can remove _monitor_chunks and its accessor methods.


vframeArrayElement::fill_in(compiledVFrame* vf, bool realloc_failures)

This allocates monitor chunks during deoptimization.
It can skip saving the MonitorChunk* in the JavaThread, but the MonitorChunk* _monitors is used locally so should stay.
(As MonitorChunks are not inserted into a list in the JavaThread, it doesn't even need a _next pointer etc...)

Incidental other use of monitor chunks:
src/hotspot/share/jfr/leakprofiler/checkpoint/rootResolver.cpp:
At a safepoint, where they were always null.  Remove this call.

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

PR Comment: https://git.openjdk.org/jdk/pull/18940#issuecomment-2075736595