[crac] RFR: Wait until G1 GC has finished before creating a snapshot. [v5]

[Jan Kratochvil]

On Fri, 28 Jul 2023 16:25:48 GMT, Sergey Nazarkin <snazarki at openjdk.org> wrote:

>> Jan Kratochvil has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Remove accidental change of UncommitInitialDelayMs.
>>    - bugreported by Radim Vansa
>
> src/hotspot/share/gc/g1/g1UncommitRegionTask.cpp line 143:
> 
>> 141:   assert(!Thread::current()->is_ConcurrentGC_thread(), "deadlock prevention");
>> 142: 
>> 143:   while (_active) {
> 
> Is this cycle safe against infinite loop? _active is not atomic or volatile.

It is true I would normally make it `volatile` (it is atomic as it is small enough), it was forgotten as other code around is using it without `volatile`. But you are right this is a different use case of `_active` where cachine may have some effect.
OTOH for my defense the `volatile` should not be required due to a memory barrier there: https://stackoverflow.com/questions/4476446/can-a-c-c-compiler-legally-cache-a-variable-in-a-register-across-a-pthread-lib

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

PR Review Comment: https://git.openjdk.org/crac/pull/93#discussion_r1278206737