RFR: 8265753: Remove manual JavaThread transitions to blocked [v2]

[Robbin Ehn]

On Tue, 11 May 2021 17:08:30 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> Robbin Ehn has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:
>> 
>>  - Merge branch 'master' into 8265753
>>  - Removed manual transitions
>
> src/hotspot/share/prims/jvmtiRawMonitor.hpp line 42:
> 
>> 40: // Important note:
>> 41: // Raw monitors can be used in callbacks which happen during safepoint by the VM
>> 42: // thread (e.g. heapRootCallback). This means we may not tranisition/safepoint
> 
> nit typo: s/e.g./e.g.,/
> nit typo: s/tranisition/transition/

Fixed

> src/hotspot/share/prims/jvmtiRawMonitor.hpp line 43:
> 
>> 41: // Raw monitors can be used in callbacks which happen during safepoint by the VM
>> 42: // thread (e.g. heapRootCallback). This means we may not tranisition/safepoint
>> 43: // poll in many cases, else the agent JavaThread can deadlock with VM thread,
> 
> nit typo: s/VM thread/the VM thread/

Fixed

> src/hotspot/share/prims/jvmtiRawMonitor.hpp line 52:
> 
>> 50: // native for all operations. However we need to honor suspend request, not
>> 51: // entering a monitor if suspended, and check for interrupts. Honoring suspened
>> 52: // and reading interrupt flag must be done from VM state (a safepoint unsafe
> 
> nit typo: s/suspend request/a suspend request/
> nit typo: s/Honoring suspened/Honoring a suspend request/
> nit typo: s/interrupt flag/the interrupt flag/
> (You'll probably want to reformat the changed lines a bit.)

Fixed

> src/hotspot/share/runtime/interfaceSupport.inline.hpp line 206:
> 
>> 204:   ~ThreadInVMfromNative() {
>> 205:     assert(!_thread->has_last_Java_frame() || _thread->frame_anchor()->walkable(), "Unwalkable stack in vm->native transition");
>> 206:     _thread->set_thread_state(_thread_in_native);
> 
> You're losing the assertion that we're transitioning from
> `_thread_in_vm` to `_thread_in_native` here. Although,
> the constructor did verify that we were in `_thread_in_native`
> when we transitioned to `_thread_in_vm` so at least the
> first half is still verified.
> 
> Can you an assertion that we're in `_thread_in_vm` here?

Fixed

> src/hotspot/share/runtime/objectMonitor.hpp line 309:
> 
>> 307:    protected:
>> 308:     ObjectMonitor* _om;
>> 309:     bool _om_exit;
> 
> Instead of `_om_exit` maybe this should be more generic?
> Perhaps `_om_op_done` or something like that?

Fixed

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

PR: https://git.openjdk.java.net/jdk/pull/3875