Misbehavior of AbstractScheduledEventExecutor.schedule in graalvm native image

[chtyim]

Expected behavior

AbstractScheduledEventExecutor.schedule should have the runnable fire at the desired future time.

Actual behavior

The runnable.run() get executed immediately

Steps to reproduce

Simply schedule a runnable with a future time, for example:

new NioEventLoopGroup().schedule(() -> System.out.println("Execute"), 10, TimeUnit.SECONDS).get();

Then compile to a binary using native-image. Execute the native-image on a different machine, and there will be a non-zero chance that the runnable would get executed immediately.

I believe the main reason is due to the AbstractScheduledEventExecutor.START_TIME was initialized at compiled time, hence getting the nanoTime on the machine where the native image built happened. Later on when it gets executed on a different machine, in the defaultCurrentTimeNanos() method, it is comparing the System.nanoTime on the current machine to the one get determined at the compile time, hence it breaks, since System.nanoTime() cannot be compared across different VM.

Current workaround

Add

 --initialize-at-run-time=io.grpc.netty.shaded.io.netty.util.concurrent.AbstractScheduledEventExecutor

to the native-image command

Proposed fix

Consider moving the START_TIME as an instance field instead of static. Alternatively, adding AbstractScheduledEventExecutor to the native-image.properties to initialize at run time.

Netty version

4.1.87.Final

JVM version (e.g. java -version)

Java version: openjdk version "17.0.2" 2022-01-18
Native image container: ghcr.io/graalvm/native-image:22.3.2

OS version (e.g. uname -a)

Native image OS: Linux 373c32b3fff1 5.15.107+ #1 SMP Sat May 6 09:12:19 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

[franz1981]

Hi @chtyim thanks for reporting it!

Consider moving the START_TIME as an instance field instead of static

I think will work although I'm not super happy about performing specific changes that make things to work for a single type of JDK (runtime behaviour, maybe but still sad, but not introducing fields), but maybe @normanmaurer or @chrisvest are fine with that.

A different approach (which performance effects are still to be evaluated and could be worse then your proposal!!) could be:
eg

final enum ScheduledEventExecutorConsts {
     Instance; 
     
     final START_TIME = System.nanoTime();          
}

it will make every scheduled executor to use just this; although some JIT (OpenJDK) optimizations will still won't work
ie static final fields are "trusted" and considered for constant folding-like optimizations -> https://github.com/openjdk/jdk11/blob/37115c8ea4aff13a8148ee2b8832b20888a5d880/src/hotspot/share/ci/ciField.cpp#L275

I didn't proposed any lazy "static" pattern, because I have no idea if native image works fine with that, but it could be a different way to solve this, if works.

[normanmaurer]

I wonder what is so bad about just adding the flag ?