Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Java Native GraalVM #562

Closed
YunaBraska opened this issue Nov 10, 2021 · 23 comments
Closed

Support for Java Native GraalVM #562

YunaBraska opened this issue Nov 10, 2021 · 23 comments
Labels
proposal Enhancement idea or proposal

Comments

@YunaBraska
Copy link
Collaborator

YunaBraska commented Nov 10, 2021

EDIT: I found a solution below in the next comment

Hi,

it would be really awesome if we could get this Nats Client running in GraalVM.
There are some features which doesn't work while compiling. For example java.security.SecureRandom
While reading in the internet, i didn't even understand how to skip this error.
even the additional parameters didn't helped: --initialize-at-run-time=io.nats.client.support.RandomUtils\,io.nats.client.support.SSLUtils\,io.nats.client.Options\,io.nats.client.NUID\,io.nats.client.Options$Builder\,io.nats.client.Options.Builder\,io.nats.client.impl.NatsConnection\,io.nats.client.impl.NatsImpl\,io.nats.client.Nats

Stacktrace:

Error: com.oracle.graal.pointsto.constraints.UnsupportedFeatureException: Detected an instance of Random/SplittableRandom class in the image heap. Instances created during image generation have cached seed values and don't behave as expected.  To see how this object got instantiated use --trace-object-instantiation=java.security.SecureRandom. The object was probably created by a class initializer and is reachable from a static field. You can request class initialization at image runtime by using the option --initialize-at-run-time=<class-name>. Or you can write your own initialization methods and call them explicitly from your main entry point.
Trace: 
        at parsing io.nats.client.support.SSLUtils.createOpenTLSContext(SSLUtils.java:43)
Call path from entry point to io.nats.client.Options$Builder.build(): 
        at io.nats.client.Options$Builder.build(Options.java:1339)
        at io.quarkus.arc.impl.AbstractSharedContext.createInstanceHandle(AbstractSharedContext.java:96)
        at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:29)
        at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:26)
        at com.oracle.svm.core.jdk.SystemPropertiesSupport.initializeLazyValue(SystemPropertiesSupport.java:216)
        at com.oracle.svm.core.jdk.SystemPropertiesSupport.getProperty(SystemPropertiesSupport.java:169)
        at com.oracle.svm.core.jdk.Target_java_lang_System.getProperty(JavaLangSubstitutions.java:287)
        at com.oracle.svm.jni.JNIJavaCallWrappers.jniInvoke_ARRAY_System_getProperty_deeeaa72a006d330408a3b7d002c7533e108bc28(generated:0)
Error: com.oracle.graal.pointsto.constraints.UnsupportedFeatureException: Detected an instance of Random/SplittableRandom class in the image heap. Instances created during image generation have cached seed values and don't behave as expected.  To see how this object got instantiated use --trace-object-instantiation=java.util.Random. The object was probably created by a class initializer and is reachable from a static field. You can request class initialization at image runtime by using the option --initialize-at-run-time=<class-name>. Or you can write your own initialization methods and call them explicitly from your main entry point.
Trace: 
        at parsing io.nats.client.NUID.<init>(NUID.java:73)
Call path from entry point to io.nats.client.impl.NatsConnection.<init>(Options): 
        at io.nats.client.impl.NatsConnection.<init>(NatsConnection.java:104)
        at io.nats.client.impl.NatsImpl.createConnection(NatsImpl.java:28)
        at io.nats.client.Nats.createConnection(Nats.java:269)
        at io.quarkus.arc.impl.AbstractSharedContext.createInstanceHandle(AbstractSharedContext.java:96)
        at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:29)
        at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:26)
        at com.oracle.svm.core.jdk.SystemPropertiesSupport.initializeLazyValue(SystemPropertiesSupport.java:216)
        at com.oracle.svm.core.jdk.SystemPropertiesSupport.getProperty(SystemPropertiesSupport.java:169)
        at com.oracle.svm.core.jdk.Target_java_lang_System.getProperty(JavaLangSubstitutions.java:287)
        at com.oracle.svm.jni.JNIJavaCallWrappers.jniInvoke_ARRAY_System_getProperty_deeeaa72a006d330408a3b7d002c7533e108bc28(generated:0)

com.oracle.svm.core.util.UserError$UserException: Unsupported features in 2 methods
Detailed message:
Error: com.oracle.graal.pointsto.constraints.UnsupportedFeatureException: Detected an instance of Random/SplittableRandom class in the image heap. Instances created during image generation have cached seed values and don't behave as expected.  To see how this object got instantiated use --trace-object-instantiation=java.security.SecureRandom. The object was probably created by a class initializer and is reachable from a static field. You can request class initialization at image runtime by using the option --initialize-at-run-time=<class-name>. Or you can write your own initialization methods and call them explicitly from your main entry point.
Trace: 
        at parsing io.nats.client.support.SSLUtils.createOpenTLSContext(SSLUtils.java:43)
Call path from entry point to io.nats.client.Options$Builder.build(): 
        at io.nats.client.Options$Builder.build(Options.java:1339)
        at io.quarkus.arc.impl.AbstractSharedContext.createInstanceHandle(AbstractSharedContext.java:96)
        at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:29)
        at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:26)
        at com.oracle.svm.core.jdk.SystemPropertiesSupport.initializeLazyValue(SystemPropertiesSupport.java:216)
        at com.oracle.svm.core.jdk.SystemPropertiesSupport.getProperty(SystemPropertiesSupport.java:169)
        at com.oracle.svm.core.jdk.Target_java_lang_System.getProperty(JavaLangSubstitutions.java:287)
        at com.oracle.svm.jni.JNIJavaCallWrappers.jniInvoke_ARRAY_System_getProperty_deeeaa72a006d330408a3b7d002c7533e108bc28(generated:0)
Error: com.oracle.graal.pointsto.constraints.UnsupportedFeatureException: Detected an instance of Random/SplittableRandom class in the image heap. Instances created during image generation have cached seed values and don't behave as expected.  To see how this object got instantiated use --trace-object-instantiation=java.util.Random. The object was probably created by a class initializer and is reachable from a static field. You can request class initialization at image runtime by using the option --initialize-at-run-time=<class-name>. Or you can write your own initialization methods and call them explicitly from your main entry point.
Trace: 
        at parsing io.nats.client.NUID.<init>(NUID.java:73)
Call path from entry point to io.nats.client.impl.NatsConnection.<init>(Options): 
        at io.nats.client.impl.NatsConnection.<init>(NatsConnection.java:104)
        at io.nats.client.impl.NatsImpl.createConnection(NatsImpl.java:28)
        at io.nats.client.Nats.createConnection(Nats.java:269)
        at io.quarkus.arc.impl.AbstractSharedContext.createInstanceHandle(AbstractSharedContext.java:96)
        at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:29)
        at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:26)
        at com.oracle.svm.core.jdk.SystemPropertiesSupport.initializeLazyValue(SystemPropertiesSupport.java:216)
        at com.oracle.svm.core.jdk.SystemPropertiesSupport.getProperty(SystemPropertiesSupport.java:169)
        at com.oracle.svm.core.jdk.Target_java_lang_System.getProperty(JavaLangSubstitutions.java:287)
        at com.oracle.svm.jni.JNIJavaCallWrappers.jniInvoke_ARRAY_System_getProperty_deeeaa72a006d330408a3b7d002c7533e108bc28(generated:0)

        at com.oracle.svm.core.util.UserError.abort(UserError.java:87)
        at com.oracle.svm.hosted.FallbackFeature.reportAsFallback(FallbackFeature.java:233)
        at com.oracle.svm.hosted.NativeImageGenerator.runPointsToAnalysis(NativeImageGenerator.java:759)
        at com.oracle.svm.hosted.NativeImageGenerator.doRun(NativeImageGenerator.java:529)
        at com.oracle.svm.hosted.NativeImageGenerator.run(NativeImageGenerator.java:488)
        at com.oracle.svm.hosted.NativeImageGeneratorRunner.buildImage(NativeImageGeneratorRunner.java:403)
        at com.oracle.svm.hosted.NativeImageGeneratorRunner.build(NativeImageGeneratorRunner.java:569)
        at com.oracle.svm.hosted.NativeImageGeneratorRunner.main(NativeImageGeneratorRunner.java:122)
        at com.oracle.svm.hosted.NativeImageGeneratorRunner$JDK9Plus.main(NativeImageGeneratorRunner.java:599)
Caused by: com.oracle.graal.pointsto.constraints.UnsupportedFeatureException: Unsupported features in 2 methods
Detailed message:
Error: com.oracle.graal.pointsto.constraints.UnsupportedFeatureException: Detected an instance of Random/SplittableRandom class in the image heap. Instances created during image generation have cached seed values and don't behave as expected.  To see how this object got instantiated use --trace-object-instantiation=java.security.SecureRandom. The object was probably created by a class initializer and is reachable from a static field. You can request class initialization at image runtime by using the option --initialize-at-run-time=<class-name>. Or you can write your own initialization methods and call them explicitly from your main entry point.
Trace: 
        at parsing io.nats.client.support.SSLUtils.createOpenTLSContext(SSLUtils.java:43)
Call path from entry point to io.nats.client.Options$Builder.build(): 
        at io.nats.client.Options$Builder.build(Options.java:1339)
        at io.quarkus.arc.impl.AbstractSharedContext.createInstanceHandle(AbstractSharedContext.java:96)
        at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:29)
        at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:26)
        at com.oracle.svm.core.jdk.SystemPropertiesSupport.initializeLazyValue(SystemPropertiesSupport.java:216)
        at com.oracle.svm.core.jdk.SystemPropertiesSupport.getProperty(SystemPropertiesSupport.java:169)
        at com.oracle.svm.core.jdk.Target_java_lang_System.getProperty(JavaLangSubstitutions.java:287)
        at com.oracle.svm.jni.JNIJavaCallWrappers.jniInvoke_ARRAY_System_getProperty_deeeaa72a006d330408a3b7d002c7533e108bc28(generated:0)
Error: com.oracle.graal.pointsto.constraints.UnsupportedFeatureException: Detected an instance of Random/SplittableRandom class in the image heap. Instances created during image generation have cached seed values and don't behave as expected.  To see how this object got instantiated use --trace-object-instantiation=java.util.Random. The object was probably created by a class initializer and is reachable from a static field. You can request class initialization at image runtime by using the option --initialize-at-run-time=<class-name>. Or you can write your own initialization methods and call them explicitly from your main entry point.
Trace: 
        at parsing io.nats.client.NUID.<init>(NUID.java:73)
Call path from entry point to io.nats.client.impl.NatsConnection.<init>(Options): 
        at io.nats.client.impl.NatsConnection.<init>(NatsConnection.java:104)
        at io.nats.client.impl.NatsImpl.createConnection(NatsImpl.java:28)
        at io.nats.client.Nats.createConnection(Nats.java:269)
        at io.quarkus.arc.impl.AbstractSharedContext.createInstanceHandle(AbstractSharedContext.java:96)
        at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:29)
        at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:26)
        at com.oracle.svm.core.jdk.SystemPropertiesSupport.initializeLazyValue(SystemPropertiesSupport.java:216)
        at com.oracle.svm.core.jdk.SystemPropertiesSupport.getProperty(SystemPropertiesSupport.java:169)
        at com.oracle.svm.core.jdk.Target_java_lang_System.getProperty(JavaLangSubstitutions.java:287)
        at com.oracle.svm.jni.JNIJavaCallWrappers.jniInvoke_ARRAY_System_getProperty_deeeaa72a006d330408a3b7d002c7533e108bc28(generated:0)

        at com.oracle.graal.pointsto.constraints.UnsupportedFeatures.report(UnsupportedFeatures.java:129)
        at com.oracle.svm.hosted.NativeImageGenerator.runPointsToAnalysis(NativeImageGenerator.java:756)
        ... 6 more

Maybe you know better what to do.

My guess is, that the class SecureRandom shouldn't be static or not public on RandomUtils Like public static final SecureRandom SRAND = new SecureRandom();

@YunaBraska
Copy link
Collaborator Author

YunaBraska commented Nov 21, 2021

I just had actually a short look into the issue. The SecureRandom is fixable while having it not static. After that GraalVM compiles everything to a native executable. But Options is using Reflections which is not allowed. so it fails on runtime while calling

instance = constructor.newInstance();

I was also able to remove this code - it looks like its only used for testing... After all of this your library is working fine on native.

These are the steps to solve GraalVM issues:

  • Refactor the static dependency of SecureRandom to non static (its nasty but possible)
  • Exchange the reflections on Options.java#L783 (only used by tests) or implement a native option wich always returns a new SocketDataPort(); instead of createInstanceOf(dataPortType)

I won't create a pull requests, since Forks costs me too much time to keep them updated :/

@scottf
Copy link
Contributor

scottf commented Nov 21, 2021

I will look into this. I'm concerned about making SecureRandom non static because of it's use in nuid, which we want to be as fast as possible and creating a secure random each time is very expensive.

I'm wondering if the initialization can be changed to lazy based on this piece of text in the error report "Or you can write your own initialization methods and call them explicitly from your main entry point." This might work, but would still come at a cost for calling a function and checking a flag.

As far as Reflection, I can't remove this code without a major version change. It's designed for someone to provide a class name to use for error listeners and connection listeners ass well as the dataPortType.

@YunaBraska
Copy link
Collaborator Author

Regarding reflection for listeners I would prefer to receive an initialised object instead of a class name which is from a specific type. Or by providing a Consumer function.
Else its optional and doesn't hurt while compiling or while not using the function except of here:

DataPort newDataPort = this.options.buildDataPort();

This destroys it while running as its not optional. Maybe one solution would be with if is default then just return new SocketDataPort()

@scottf
Copy link
Contributor

scottf commented Nov 21, 2021

That's a simple enough change maybe this:

public DataPort buildDataPort() {
    if (dataPortType.equals(DEFAULT_DATA_PORT_TYPE)) {
        return new SocketDataPort();
    }
    return (DataPort) Options.Builder.createInstanceOf(dataPortType);
}

In the next major version we should have some factory mechanism like spring.

@dmadunic
Copy link

dmadunic commented Jan 6, 2023

Any progress on this issue? It would really be great if Nats client could work with GraalVM.

@turing85
Copy link

turing85 commented Mar 8, 2023

Hi!
So the point with the static field is fixable by some lazy init code (which involves double-checked locking (baeldung.com), but the double-check should be hit seldomly. The code would look something like this:

    private static final Object S_RAND_LOCK = new Object();
    private static SecureRandom sRand;

    public static SecureRandom getSRand() {
        if (sRand == null) {
            synchronized (S_RAND_LOCK) {
                if (sRand == null) {
                    sRand = new SecureRandom();
                }
            }
        }
        return sRand;
    }

Here is a patchfile with the necessary changes (including the change for PRAND, as well as updates to all calling sides)

(Whether the dedicated Objects for locking are really necessary, or the RandomUtils.class object should be used is of course debatable, this is just a rough sketch)

If this code looks fine just say the word, and I will open an PR.


With regards to reflection: Removing it is great, but not necessary. There is also the possibility to register the classes in question for reflection, and add them to the official metadata repository (github.com).

@turing85
Copy link

turing85 commented Mar 8, 2023

As an aside: I noticed that NKey and NUID use PRAND (= java.util.Random). Is this intentional, or would a ThreadLocalRandom be better a fit?

public void clear() {
if (privateKeyAsSeed != null) {
for (int i=0; i< privateKeyAsSeed.length ; i++) {
privateKeyAsSeed[i] = (char)(PRAND.nextInt(26) + 'a');
}
Arrays.fill(privateKeyAsSeed, '\0');
}
if (publicKey != null) {
for (int i=0; i< publicKey.length ; i++) {
publicKey[i] = (char)(PRAND.nextInt(26) + 'a');
}
Arrays.fill(publicKey, '\0');
}
}

public NUID() {
// Generate a cryto random int, 0 <= val < max to seed pseudorandom
seq = nextLong(PRAND, maxSeq);
inc = minInc + nextLong(PRAND, maxInc - minInc);
pre = new char[preLen];
for (int i = 0; i < preLen; i++) {
pre[i] = '0';
}
randomizePrefix();
}

void resetSequential() {
seq = nextLong(PRAND, maxSeq);
inc = minInc + nextLong(PRAND, maxInc - minInc);
}

@bruth bruth added the proposal Enhancement idea or proposal label Aug 19, 2023
@scottf scottf closed this as completed Mar 26, 2024
@prasannak-ra
Copy link

@scottf - sorry probably silly question, does the closed status mean, the patch discussed in above comments is merged and we can build GraalVM with this lib?

@scottf
Copy link
Contributor

scottf commented Apr 5, 2024

@prasannak-ra

Based on this note:

These are the steps to solve GraalVM issues:

  • Refactor the static dependency of SecureRandom to non static (its nasty but possible)
  • Exchange the reflections on Options.java#L783 (only used by tests) or implement a native option wich always returns a new SocketDataPort(); instead of createInstanceOf(dataPortType)

I made this PR: #1022

I fixed the issue with the SocketDataPort using reflection by providing the class directly with new instead of Class.forName

I did not fix the issue of SecureRandom static dependency. The main reason is I want it static because it's used in NUID creation which needs to be fast as possible. Can someone explain to me this is an issue? Does GraalVM not like SecureRandom or is it the static aspect of it. I'm having a hard time grokking why being static could possibly be the issue.

Honestly, I don't know why this needs to be "secure" as long as it's pseudo random with a good seed, the use should be fine. It's possible the original developer just figured use secure to be safe.

You might need to just try it our. If you have comments/knowledge etc. please let me know.

@prasannak-ra
Copy link

Thanks @scottf - let me give it a try.
Based on the error message, I think graal does not like static aspect of it, it implies that seed value can be cached at the time of image generation, so it does not really stay random anymore - just a hunch.

Regards,
Prasanna

@turing85
Copy link

turing85 commented Apr 5, 2024

@prasannak-ra you are spot on. This might be preventable by instructing GraalVM to recalculate the field values at image start time, we did something similar here: https://github.com/quarkiverse/quarkus-artemis/blob/main/core/runtime/src/main/java/io/quarkus/artemis/core/runtime/graal/JGroupSubstitutions.java

The relevant documentation can be found here:

@scottf
Copy link
Contributor

scottf commented Apr 5, 2024

@YunaBraska
Copy link
Collaborator Author

YunaBraska commented Apr 5, 2024

Could be solve able by adding “–initialize-at-run-time=java.security.SecureRandom” when building the project with this lib

@turing85
Copy link

turing85 commented Apr 5, 2024

Not sure about this. Those are parameters for the native-image command. We do not control this command since it is executed on the user side, when the application is built natively.

@prasannak-ra
Copy link

Could be solve able by adding “–initialize-at-run-time=java.security.SecureRandom” when building the project with this lib

Thanks @YunaBraska - this is in addition to earlier arguments you mentioned in first comment? (--initialize-at-run-time=io.nats.client.support.RandomUtils,io.nats.client.support.SSLUtils,io.nats.client.Options,io.nats.client.NUID,io.nats.client.Options$Builder,io.nats.client.Options.Builder,io.nats.client.impl.NatsConnection,io.nats.client.impl.NatsImpl,io.nats.client.Nats)

Or only –initialize-at-run-time=java.security.SecureRandom worked?
I'll give both a try either way.

@YunaBraska
Copy link
Collaborator Author

@prasannak-ra: I have created an example build with Nats and Graalvm nats-graalvm-example
@turing85 currently, there is no other way than adding --initialize-at-run-time=io.nats.client.support.RandomUtils --initialize-at-run-time=java.security.SecureRandom to the build. Maybe you can refer the userst to the nats-graalvm-example repo for more understanding.

I can add more description to the repository, if something is unclear.

@turing85
Copy link

turing85 commented Apr 8, 2024

@prasannak-ra: I have created an example build with Nats and Graalvm nats-graalvm-example @turing85 currently, there is no other way than adding --initialize-at-run-time=io.nats.client.support.RandomUtils --initialize-at-run-time=java.security.SecureRandom to the build. Maybe you can refer the userst to the nats-graalvm-example repo for more understanding.

I can add more description to the repository, if something is unclear.

I spoke with @zakkak, he recommend reading https://foivos.zakkak.net/tutorials/working-with-randoms-native-images/#headline-7. As outlined in the article, we have three options:

  • setting --initialize-at-run-time
  • writing a custom feature to runtime-initialize the class
  • provide a substitution

@prasannak-ra
Copy link

Thanks @YunaBraska, @turing85 - I'll try all the approaches mentioned and post my findings here.

@zakkak
Copy link

zakkak commented Apr 8, 2024

Please also consider what @turing85 suggested in #562 (comment) about altering the source code to avoid the issue in the first place instead of trying to instruct GraalVM how to handle the issue.

@YunaBraska
Copy link
Collaborator Author

@zakkak regarding @turing85 suggestion in #562 (comment)
I can understand and also proposed it. We took the consideration of lazy loading the random seeds, but these parts need to be highly performant, therefore it's unlikely that there will be code changes as then there is an additional null check at every usage. @scottf correct me if I am wrong.

The issue is, that GraalVM stopps the build instead of just warn the user and just add these parameters by itself as the error is coming from the GraalVM analyzer as a soft error.

@zakkak
Copy link

zakkak commented Apr 8, 2024

@YunaBraska looking at the error log in #562 (comment) it looks like this is being triggered when using Quarkus. Have you tried compiling the project to native outside of a Quarkus context?

I am asking because Quarkus explicitly initializes everything at build time, but that's not the default for GraalVM. That said, the issue might not manifest outside of a Quarkus project. In that case the right thing to do would be to create a Quarkus extension (or augment an existing one).

@YunaBraska
Copy link
Collaborator Author

I have no idea about quarkus but looks good so far there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Enhancement idea or proposal
Projects
None yet
Development

No branches or pull requests

7 participants