[crac] RFR: Correct System.nanotime() value after restore [v2]

[Radim Vansa]

On Thu, 11 May 2023 14:39:21 GMT, Jan Kratochvil <duke at openjdk.org> wrote:

>>> Since you mention it was a bug in kernel or criu and it has been almost 3 years since your commit, may be it is worth enabling the criu changes again to see if the timedwait problem still exists, unless you have already done that.
>> 
>> AFAIK the bug is fixed, but I see no point of relying on OS here. Is there one? Timens that is not changed by CRIU provides correct values for our nanoTime() [1].
>> 
>>> The value returned represents nanoseconds since some fixed but arbitrary origin time (perhaps in the future, so values may be negative). The same origin is used by all invocations of this method in an instance of a Java virtual machine
>> 
>> [1] https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/System.html#nanoTime()
>
> Upstream criu does provide the time namespace as stated by @AntonKozlov:
> 
> CLOCK_MONOTONIC=301.735134591 CLOCK_BOOTTIME=301.735155494
> CLOCK_MONOTONIC=302.735345917 CLOCK_BOOTTIME=302.735358109
> Warn  (compel/arch/x86/src/lib/infect.c:356): Will restore 7757 with interrupted system call
> [1]+  Killed                  ./clock_gettime
> restore:
> CLOCK_MONOTONIC=302.803360137 CLOCK_BOOTTIME=302.803373299
> restore:
> CLOCK_MONOTONIC=302.806677876 CLOCK_BOOTTIME=302.806696098
> 
> I do not see why JVM should reimplement what CRIU already does. One can solve that in the future when CRaC is really going to run on non-Linux system. One will need to port or reimplement there CRIU in the first place.

@jankratochvil The problem with 'hanging' restore is actually not specific to Fedora at all; from the description I thought that it is unresponsive in native part, but `jstack` tells that it was your testcase sleeping in `Thread.sleep()`. Turns out `os::PlatformEvent::park` creates an absolute timestamp on monotonic time and calls `pthread_cond_timedwait` - when the time runs backward during restore this does not get reevaluated. Based on the codepath I think that this would not be limited to `Thread.sleep()` but many other 'waiting' synchronization primitives. 

While I consider this a serious issue (and I'll be thinking about a solution), I think that it's independent from this PR - this handles the value we're reading directly through `System.nanoTime()`. I'll also test whether it happens when timens is used by CRIU.

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

PR Comment: https://git.openjdk.org/crac/pull/53#issuecomment-1552571027