On Fri, 28 May 2021 13:25:23 GMT, Martin Doerr <[email protected]> wrote:
>> We need a fix for crashes in get_current_contended_monitor due to concurrent
>> modification of memory locations which are not declared volatile. See bug
>> for details.
>
> Martin Doerr has updated the pull request incrementally with one additional
> commit since the last revision:
>
> whitespace fix
Thumbs up. Just a couple of suggestions about the comments.
src/hotspot/share/runtime/thread.hpp line 757:
> 755: ObjectMonitor* current_pending_monitor() {
> 756: // Using atomic load to prevent compilers from reloading
> (ThreadService::get_current_contended_monitor).
> 757: // In case of concurrent modification, reloading pointer after NULL
> check must be prevented.
Perhaps rewrite the comment like this:
// Use Atomic::load() to prevent data race between concurrent modification and
// concurrent readers, e.g., ThreadService::get_current_contended_monitor().
src/hotspot/share/runtime/thread.hpp line 770:
> 768: }
> 769: ObjectMonitor* current_waiting_monitor() {
> 770: // Using atomic load as in current_pending_monitor.
Perhaps:
`// See the comment in current_pending_monitor() above.`
-------------
Marked as reviewed by dcubed (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/4224
