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

[Radim Vansa]

On Wed, 26 Jul 2023 16:45:25 GMT, Jan Kratochvil <jkratochvil at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/g1UncommitRegionTask.cpp line 144:
>> 
>>> 142: }
>>> 143: 
>>> 144: void G1UncommitRegionTask::wait_if_active() {
>> 
>> I guess this gets thread-safe because `_instance` is set in VM thread, and we're calling it from another VM operation. However a comment, and preferably an assertion on which thread is executing this would be helpful in the future.
>
> I have added there some assertions. Not sure they should stay this way, do you have more insight about it?

Oh, I looked wrong, we're *not* calling it from VM operation (`VM_Crac::doit`). Asserting on the name of the thread makes no sense, that can be changed arbitrarily, and the checkpoint operation can be triggered from any thread, including a non-application thread if we use the `jcmd <pid> JDK.checkpoint`.

I haven't checked all code paths but it seems that the `_instance` is set from `VM_G1CollectFull`. If we are to skip synchronization of access to `_instance` we could call the `wait_if_active` from our `VM_Crac` and use `assert(Thread::current()->is_VM_thread())` as the assertion. However when we're in another VM operation I guess the GC thread we would be waiting for isn't suspended in a safepoint (so we would deadlock).

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

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