RFR: 8273695: Safepoint deadlock on VMOperation_lock [v3]

[Patricio Chilano Mateo]

On Fri, 24 Sep 2021 16:48:38 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 incrementally with one additional commit since the last revision:
> 
>   Fixed spelling

LGTM!

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

> 88:   // 3: The JavaThread wakes-up from blocked and does not allow suspend.
> 89:   // We are now about to avoid processing and thus no cross modify fence will be executed.
> 90:   // Therefore we execute it here.

I would just say we still need to execute this fence in case a safepoint happened while being blocked, and not mention step 1 since as commented above there might be nothing to process.

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

Marked as reviewed by pchilanomate (Committer).

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