RFR: 8314225: SIGSEGV in JavaThread::is_lock_owned [v2]

[David Holmes]

On Wed, 1 May 2024 08:39:35 GMT, Dean Long <dlong at openjdk.org> wrote:

>> Kevin Walls has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Feedback from Dean
>
> src/hotspot/share/runtime/vframeArray.cpp line 100:
> 
>> 98: 
>> 99:           assert(current_thread->is_Java_thread(), "Must be a JavaThread");
>> 100:           assert(ObjectSynchronizer::current_thread_holds_lock((JavaThread*) current_thread, Handle(current_thread, dest->obj())),
> 
> This makes me wonder about the assert at line 96 that allows monitor->owner() == nullptr.  If that can happen due to OOM, then we need to check for that here too.

Nit: don't use C-style casts use `JavaThread::cast(thread_current)`  (though this won't be necessary once you change the type of `current_thread`.

> src/hotspot/share/runtime/vframeArray.cpp line 317:
> 
>> 315:     BasicObjectLock* src = _monitors->at(index);
>> 316:     top->set_obj(src->obj());
>> 317:     assert(ObjectSynchronizer::current_thread_holds_lock(thread, Handle(thread, src->obj())), "should be held, before move_to");
> 
> Same comment as above, may need to check for null obj.

Not sure how `obj` can be null in this code. ???

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18940#discussion_r1586088980
PR Review Comment: https://git.openjdk.org/jdk/pull/18940#discussion_r1586116316