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

Using asSequence.chunked(...) causes native mode to fail #13103

Open
arturs-cc opened this issue Nov 4, 2020 · 15 comments
Open

Using asSequence.chunked(...) causes native mode to fail #13103

arturs-cc opened this issue Nov 4, 2020 · 15 comments
Assignees
Labels
area/kotlin kind/bug Something isn't working

Comments

@arturs-cc
Copy link

arturs-cc commented Nov 4, 2020

Describe the bug
Native mode fails to run when using .asSequence().chunked(...) for some reason. Problem doesn't exist when using JVM

First thing that happens is actually something that has been seen in other issues before:

22:12:51 [] ERROR [io.qu.application] (main) Failed to start application (with profile test): java.lang.IllegalStateException: @NotNull method kotlin/reflect/jvm/internal/impl/builtins/KotlinBuiltIns.getBuiltInClassByFqName must not return null 

Similar/Same issue has been seen here (however seemingly not related to this):
oracle/graal#2306
#8479 (comment)
#3954 (comment)

Now if we apply configuration suggested in oracle/graal#2306 we get further.
Suddenly we will need to add classes to reflection that weren't required before.
For example if we have a class com.example.LambdaRequest which is already registered for reflection using @RegisterForReflection it will require to add com.example.LambdaRequest[] to reflection config as well. Same goes for all the fields in LambdaRequest.
As well as needing their array counterparts added to reflection config it also will require java.util.Set[] and java.util.List[] to added as well.

If we do all of the above we get further.
Now all of a sudden we get this:

00:15:05 [] ERROR [io.qu.application] (main) Failed to start application (with profile test): java.lang.ClassNotFoundException: java.lang.Enum 

Now if we add java.lang.Enum to reflection-config as well (which is quite bizarre in my opinion) we get further
At this point application starts up fine. But now jackson doesn't recognise the difference between nullable and non nullable fields in kotlin any more. So even if field is nullable. Adding required = false doesn't help, assigning default values doesn't help and nothing else seems to work either which suggests to me that the issue lies deeper.

At this point it looks to me that the root issue is behind the first error.

Reproducer:
https://github.com/arturs-cc/quarkus-native-kotlin-chunked-issue

Environment (please complete the following information):

  • Output of uname -a or ver: Darwin localhost 19.6.0 Darwin Kernel Version 19.6.0: Thu Jun 18 20:49:00 PDT 2020; root:xnu-6153.141.1~1/RELEASE_X86_64 x86_64
  • Output of java -version: openjdk version "11.0.3" 2019-04-16
    OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.3+7)
    OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.3+7, mixed mode)
  • Kotlin version - 1.3.72
  • aws sdk version: 2.11.10
  • Quarkus version or git rev: 1.8.0.Final
  • Build tool (ie. output of mvnw --version or gradlew --version):
    Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
    Maven home: /usr/local/Cellar/maven/3.6.3_1/libexec
    Java version: 14.0.1, vendor: N/A, runtime: /usr/local/Cellar/openjdk/14.0.1/libexec/openjdk.jdk/Contents/Home
    Default locale: en_GB, platform encoding: UTF-8
    OS name: "mac os x", version: "10.15.6", arch: "x86_64", family: "mac"

Happy to answer any questions or provide more information!

@arturs-cc arturs-cc added the kind/bug Something isn't working label Nov 4, 2020
@quarkusbot
Copy link

/cc @evanchooly

@arturs-cc arturs-cc changed the title Using Async S3 client causes cascading set problems with native mode in a kotlin project Using asSequence.chunked(...) causes native mode to fail Nov 9, 2020
@arturs-cc
Copy link
Author

Has anyone had a chance to look at this? :)
Having some builtin functions fail in native mode is not great

@sherl0cks
Copy link
Contributor

It's a real pain @arturs-cc . I think there needs to be a quarkus kotlin-jackson extension that handles all of these nasties for you.

@miron4dev
Copy link

I updated reflection-config.json and resources-config.json in the mentioned repo

reflection-config.json:

[
  {
    "name": "kotlin.reflect.jvm.internal.ReflectionFactoryImpl",
    "allDeclaredConstructors":true
  },
  {
    "name": "kotlin.KotlinVersion",
    "allPublicMethods": true,
    "allDeclaredFields":true,
    "allDeclaredMethods":true,
    "allDeclaredConstructors":true
  },
  {
    "name": "kotlin.KotlinVersion[]"
  },
  {
    "name": "kotlin.KotlinVersion$Companion"
  },
  {
    "name": "kotlin.KotlinVersion$Companion[]"
  }
]

resources-config.json:

{
  "resources": [
    {
      "pattern": ".*\\.xml$"
    },
    {
      "pattern": ".*\\.json$"
    },
    {
      "pattern": ".*\\.properties$"
    },
    {"pattern":"META-INF/.*.kotlin_module$"},
    {"pattern":"META-INF/services/.*"},
    {"pattern":".*.kotlin_builtins"}
  ]
}

and now started receiving the new error:

{"errorType":"com.fasterxml.jackson.databind.exc.InvalidDefinitionException","errorMessage":"Class com.example.LambdaRequest[] is instantiated reflectively but was never registered. Register the class by using org.graalvm.nativeimage.hosted.RuntimeReflection\n at [Source: (sun.net.www.protocol.http.HttpURLConnection$HttpInputStream); line: 1, column: 1]"}

I don't know why it requires LambdaRequest[] (array) and not LambdaRequest, but looks like @RegisterForReflection doesn't handle this case.
The issue can be solved by manually adding the required class to reflection-config.json

{
    "name": "com.example.LambdaRequest[]",
    "allPublicMethods": true,
    "allDeclaredFields":true,
    "allDeclaredMethods":true,
    "allDeclaredConstructors":true
  }

It is ok as a workaround, but I think @RegisterForReflection has to cover this use case and quarkus-kotlin extension has to register required classes and resources.

@miron4dev
Copy link

Probably this is a separate issue, but when I explicitly registered LambdaRequest[] in reflection-config.json and called lambda I started receiving the new error:

2020-11-19 09:46:24,012 ERROR [io.qua.ama.lam.run.AbstractLambdaPollLoop] (Lambda Thread) Failed to run lambda: com.fasterxml.jackson.module.kotlin.MissingKotlinParameterException: Instantiation of [simple type, class com.example.LambdaRequest] value failed for JSON property nullProperty due to missing (therefore NULL) value for creator parameter null which is a non-nullable type
 at [Source: (sun.net.www.protocol.http.HttpURLConnection$HttpInputStream); line: 4, column: 1] (through reference chain: com.example.LambdaRequest["nullProperty"])
        at com.fasterxml.jackson.module.kotlin.KotlinValueInstantiator.createFromObjectWith(KotlinValueInstantiator.kt:112)
        at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.build(PropertyBasedCreator.java:198)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:422)
        at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1287)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:326)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:159)
        at com.fasterxml.jackson.databind.ObjectReader._bindAndClose(ObjectReader.java:1719)
                at com.fasterxml.jackson.databind.ObjectReader.readValue(ObjectReader.java:1228)
        at io.quarkus.amazon.lambda.runtime.AbstractLambdaPollLoop$1.run(AbstractLambdaPollLoop.java:78)
        at java.lang.Thread.run(Thread.java:834)
        at com.oracle.svm.core.thread.JavaThreads.threadStartRoutine(JavaThreads.java:517)
        at com.oracle.svm.core.posix.thread.PosixJavaThreads.pthreadStartRoutine(PosixJavaThreads.java:192)

This might be ok for non-nullable fields, but nullProperty has a nullable type!

package com.example

import com.fasterxml.jackson.annotation.JsonCreator
import com.fasterxml.jackson.annotation.JsonProperty
import io.quarkus.runtime.annotations.RegisterForReflection

@RegisterForReflection
data class LambdaRequest @JsonCreator constructor(

    @JsonProperty(value = "nullProperty")
    val nullProperty: String?,

    @JsonProperty(value = "nonNullProperty")
    val nonNullProperty: String
)

@miron4dev
Copy link

miron4dev commented Nov 19, 2020

Kotlin chunked uses yield internally, and yield is a suspendable function (part of coroutines API).

As far as I understand, Quarkus doesn't support coroutines

#10162
#7999

@sherl0cks
Copy link
Contributor

sherl0cks commented Nov 19, 2020

Probably this is a separate issue, but when I explicitly registered LambdaRequest[] in reflection-config.json and called lambda I started receiving the new error:

2020-11-19 09:46:24,012 ERROR [io.qua.ama.lam.run.AbstractLambdaPollLoop] (Lambda Thread) Failed to run lambda: com.fasterxml.jackson.module.kotlin.MissingKotlinParameterException: Instantiation of [simple type, class com.example.LambdaRequest] value failed for JSON property nullProperty due to missing (therefore NULL) value for creator parameter null which is a non-nullable type
 at [Source: (sun.net.www.protocol.http.HttpURLConnection$HttpInputStream); line: 4, column: 1] (through reference chain: com.example.LambdaRequest["nullProperty"])
        at com.fasterxml.jackson.module.kotlin.KotlinValueInstantiator.createFromObjectWith(KotlinValueInstantiator.kt:112)
        at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.build(PropertyBasedCreator.java:198)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:422)
        at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1287)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:326)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:159)
        at com.fasterxml.jackson.databind.ObjectReader._bindAndClose(ObjectReader.java:1719)
                at com.fasterxml.jackson.databind.ObjectReader.readValue(ObjectReader.java:1228)
        at io.quarkus.amazon.lambda.runtime.AbstractLambdaPollLoop$1.run(AbstractLambdaPollLoop.java:78)
        at java.lang.Thread.run(Thread.java:834)
        at com.oracle.svm.core.thread.JavaThreads.threadStartRoutine(JavaThreads.java:517)
        at com.oracle.svm.core.posix.thread.PosixJavaThreads.pthreadStartRoutine(PosixJavaThreads.java:192)

Yea this is a pain too. For nullable fields, I've gotten around this by making sure there is a default set for the val. @miron4dev are you on Java 8 or 11? The solution in #3954 does not work for me in Java 8, but I'm trying it now on Java 11 and will report back.

Kotlin chunked uses yield internally, and yield is a suspendable function (part of coroutines API).

As far as I understand, Quarkus doesn't support coroutines

#10162
#7999

Im using coroutines fine in production apart from the pain of this serialization issue. Vert.x coroutine support is great for IO.

@dforsl
Copy link

dforsl commented Nov 21, 2020

Also want to raise this one, as it's a real bummer for native and kotlin. We're seeing the same reflection-issues with native and kotlin with coroutines (and tried, as mentioned above, the different configs etc). We've thus had to abandon coroutines for the time being in all of our quarkus-based services and lambdas - as we run almost everything as native builds. Last version we tried it on was 1.9.2.Final with Java 11 and GraalVM 20.2.0.

@evanchooly evanchooly self-assigned this Nov 24, 2020
@evanchooly
Copy link
Member

fwiw, i can't even recreate the failure. i get this when I run it:

 ./invoke.sh
Invoking not.used.in.provided.runtimei (provided)
Decompressing /Users/julee/Downloads/reproducers/quarkus-native-kotlin-chunked-issue/target/function.zip
Skip pulling image and use local one: amazon/aws-sam-cli-emulation-image-provided:rapid-1.13.2.

Mounting /private/var/folders/0m/6mjfxngx2n386tgk9rtxwcv80000gn/T/tmpsa49oeyh as /var/task:ro,delegated inside runtime container
No response from invoke container for RiskScoreEngineNativeFunction

@sherl0cks
Copy link
Contributor

@evanchooly how are you trying to reproduce? The error you are showing would indicate to me the lambda container crashing and thus a possible successful recreation. Happy to partner with you on this issue to get a solution.

One thought by the way, now that master has support for kotlin 1.4, is to put an interface around serialization in the lambda integration (it's currently hard coded to jackson). Provide a default bean for jackson, but allow users to provide a kotlin serialization implementation. This side steps the whole problem space (Jackson and reflection) by going to a more native solution (kotlin and compile serialization code gen).

@evanchooly
Copy link
Member

I'm literally cutting and pasting the instructions above. I had initially bumped everything to the latest versions but when I didn't get that error I rolled back to what you've provided and still don't get the error.

@sherl0cks
Copy link
Contributor

Hm ok this isn't my issue/reproducer but @arturs-cc 's. I'll take a look today to see if I can get that reproducer working. I have this issue on ~10 repos right now on a variety of graal versions, so I know its real.

In the meantime - thoughts about supporting kotlin serialization? It seems to align well with quarkus / graal aot goals and eliminates this whole class of reflection issues.

@arturs-cc
Copy link
Author

@evanchooly
Looks like the issue you are facing is because of the sam-cli version. Right now it works on 1.7 and doesn't for 1.13.2 for no apparent reason.
If you can - could you please downgrade to 1.7 where it works? Or just wait for a couple days while i investigate and fix it for 1.13.2. :)
As of right now it indeed fails with No response from invoke container for QuarkusNativeChunkedIssue

@mar9000
Copy link

mar9000 commented Feb 23, 2021

I had a similar problem with the same workaround to solve the problem: use reflect-config.json.
While I was trying to create an extension for BoofCV I came to a point where even with a buildStep that register the class name "..... $Corner[]" (and also ".....$Corner" and many more names) the native version fired an exception saying to register the very same name for reflection. Put "..... $Corner[]" into reflect-config.java solved the problem.
The exception got reised when the code try to use reflect.Array.newInstance().

If you think this is the same problem and you are interested in reproducing the error, I can upload my BoofCV extension.

@mar9000
Copy link

mar9000 commented Feb 23, 2021

If not I would probably open another issue because this one is the only one I've found related to registration of a "classname[]" class name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kotlin kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants