On Wed, 14 Apr 2021 16:05:39 GMT, Daniel D. Daugherty <dcu...@openjdk.org>
wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fixed flag undef dep + spelling error
>
> src/hotspot/share/runtime/handshake.hpp line 99:
>
>> 97: // but we need to check for a safepoint before.
>> 98: // (this is due to a suspension handshake which put the JavaThread in
>> blocked
>> 99: // state so a safepoint may be in-progress)
>
> nit: s/(this/(This/
> nit: s/in-progress)/in-progress.)/
Fixed
> src/hotspot/share/runtime/handshake.hpp line 166:
>
>> 164: // thread gets suspended again (after a resume)
>> 165: // and we have not yet processed it.
>> 166: bool _async_suspend_handshake;
>
> Does this need to be `volatile`?
No, _async_suspend_handshake is now only loaded/stored with mutex held.
> src/hotspot/share/runtime/handshake.hpp line 177:
>
>> 175: void set_suspended(bool to) { return
>> Atomic::store(&_suspended, to); }
>> 176: bool has_async_suspend_handshake() { return
>> _async_suspend_handshake; }
>> 177: void set_async_suspend_handshake(bool to) { _async_suspend_handshake
>> = to; }
>
> Does this need to be an Atomic::store?
No, _async_suspend_handshake is now only loaded/stored with mutex held.
> src/hotspot/share/runtime/objectMonitor.cpp line 424:
>
>> 422: // thread that suspended us.
>> 423: _recursions = 0;
>> 424: _succ = NULL;
>
> You might want to add a comment here:
> // Don't need a full fence after clearing successor here because of the call
> to exit().
Fixed
-------------
PR: https://git.openjdk.java.net/jdk/pull/3191
