RFR: 8314225: SIGSEGV in JavaThread::is_lock_owned

[Kevin Walls]

On Thu, 25 Jan 2024 22:10:01 GMT, Dean Long <dlong at openjdk.org> wrote:

>> JavaThread's _monitor_chunks member is temporary storage used by deoptimization.
>> When other threads inspect it using JavaThread::monitor_chunks(), if it is non-null that means a deoptimization is in progress, and the value will be removed shortly.
>> 
>> There are a few places where we attempt to follow the MonitorChunk*, but that would only be valid if deopt is in progress, and only safe if we could know the deopt is not going to complete.  But that the deopt will complete, and will free the MonitorChunks and clear the value.  So this is rare but there is a race and a risk of following a MonitorChunk* as it gets freed, and crashing.
>
> src/hotspot/share/runtime/javaThread.cpp line 1038:
> 
>> 1036: // A ThreadLocalHandshake will mean deopt is complete.
>> 1037: MonitorChunk* JavaThread::monitor_chunks_safe() const {
>> 1038:   MonitorChunk* chunks = _monitor_chunks;
> 
> There's still a race here, right?  The target thread isn't necessarily the same as the current thread, so it could deoptimize immediately after reading `_monitor_chunks`.  I think for correctness it would always need to handshake.  But then what's the point, if we always return null?  Then this just turns into a strange synchronization mechanism.

Right, target thread is commonly not the current thread (hence the occasional problem).

At the moment yes what I have here is a strange synchronziation, waiting for a thing to be null, then returning null.
It's a step towards removing the callers of monitor_chunks() from the few places that are not deoptimization itself.  Only waiting really to do the assert.  As you notice the call from rootResolver.cpp looks safe to remove: I see it is called from a safepoint.

In this one in javaThread.cpp, yes it could deoptimize and set _monitor_chunks after we read it.  If it does that...  I'm checking into what can happen...

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17566#discussion_r1468164254