RFR: 8267842: SIGSEGV in get_current_contended_monitor [v5]

[Daniel D.Daugherty]

On Fri, 28 May 2021 13:25:23 GMT, Martin Doerr <mdoerr at openjdk.org> 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