RFR: 8273695: Safepoint deadlock on VMOperation_lock [v2]

[Daniel D.Daugherty]

On Wed, 22 Sep 2021 11:50:46 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> We should not do any processing in SM::should_process().
>> The query is used to determine if we need to process safepoint/handshakes and with this change StackWaterMark.
>> When locking a Mutex which may be acquired in such processing, we must release that Mutex before we can start processing, otherwise we can deadlock.
>> 
>> This change adds a method to determine if StackWaterMarkSet::on_safepoint() will do any processing.
>> In that case there are poll is armed, we do not allow suspend handshakes and there is no safepoint and no non-suspend handshakes, we still return true if StackWaterMarkSet needs processing.
>> Thus the code querying should release any such Mutex and call process SM::process_if_requested().
>> 
>> The cross_modify_fence() do not have any such state, so we still need to emit that before returning false if poll is armed.
>> 
>> Passes t1-t4 and local stressing.
>
> 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 four additional commits since the last revision:
> 
>  - Merge branch 'master' into 8273695-swm-should-proc
>  - Added update poll values and fixed PR comments
>  - Removed string dedup test from problem list
>  - Check SWS for processing

Thumbs up.

Minor typographical nits.

src/hotspot/share/runtime/safepointMechanism.inline.hpp line 79:

> 77:   }
> 78: 
> 79:   // It have boiled down to two possibilities.

Please change to:
// It has boiled down to two possibilities:

src/hotspot/share/runtime/safepointMechanism.inline.hpp line 82:

> 80:   // 1: We have nothing to process, this just a disarm poll.
> 81:   // 2: We have a suspend handshake, which cannot be processed.
> 82:   // We update the poll value in case of a disarm, to reduce false positive.

Typo: s/positive/positives/

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

Marked as reviewed by dcubed (Reviewer).

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