RFR: 8271348: Add stronger sanity check of thread state when polling for safepoint/handshakes [v2]

Daniel D.Daugherty dcubed at openjdk.java.net
Fri Jul 30 16:11:32 UTC 2021


On Fri, 30 Jul 2021 01:10:03 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> So I think @robehn was trying to separate these two lines of code:
>> 
>> `uint64_t safepoint_id = SafepointSynchronize::safepoint_counter();`
>> 
>> and
>> 
>> `thread->safepoint_state()->set_safepoint_id(safepoint_id);`
>> 
>> I think the same situation applies in the updated code, but it is harder
>> to see in this GitHub view. Especially now that I've added all these
>> comments.
>
> Thanks and apologies for the digression. I'm assuming the intended required order is:
> - load safepoint counter into safepoint_id
> - load thread safepoint state
> - store safepoint_id into safepoint_state
> 
> But the store is a release_store, so it is effectively preceded by a LoadStore|SoreStore barrier. So both loads must come before the store. The loads themselves could be reordered AFAICS but with no affect on correctness. So I remain unclear about the "load dependent store" comment actually relates to.
> Oh well, not a problem for this PR.

Okay so we're done with this one for this PR.

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

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


More information about the hotspot-runtime-dev mailing list