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

Quarkus, Jackson and Kotlin 1.4 #11549

Closed
kny78 opened this issue Aug 24, 2020 · 49 comments · Fixed by #11970 or #13445
Closed

Quarkus, Jackson and Kotlin 1.4 #11549

kny78 opened this issue Aug 24, 2020 · 49 comments · Fixed by #11970 or #13445
Assignees
Labels
area/core kind/bug Something isn't working
Milestone

Comments

@kny78
Copy link
Contributor

kny78 commented Aug 24, 2020

Describe the bug

  • Upgraded to Kotlin 1.4
  • Use Jackson and Jackson-module-kotlin
  • Running with @QuarkusTest fails
  • Same test without @QuarkusTest is ok

Se repository: https://github.com/kny78/quarkus-kotlin14-jackson-fail
See failure output: https://github.com/kny78/quarkus-kotlin14-jackson-fail/tree/master/failure-logs

Expected behavior
Both tests should run ok:

  • im.kny.DeSerializeQuarkusTest (Fails)
  • im.kny.DeSerializeNonQuarkusTest (Ok)

Actual behavior
The test im.kny.DeSerializeQuarkusTest fail.s

To Reproduce
Steps to reproduce the behavior:

  1. git clone git@github.com:kny78/quarkus-kotlin14-jackson-fail.git
  2. mvnw install

**Environment (please complete the following information):**
 - Output of `uname -a` or `ver`:

        Linux kjetil 5.7.15-200.fc32.x86_64 #1 SMP Tue Aug 11 16:36:14 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

 - Output of `java -version`: 

        openjdk version "11.0.8" 2020-07-14
        OpenJDK Runtime Environment 18.9 (build 11.0.8+10)
        OpenJDK 64-Bit Server VM 18.9 (build 11.0.8+10, mixed mode, sharing)

 - GraalVM version (if different from Java): N/A
 - Quarkus version or git rev: 1.7.0
 - Build tool (ie. output of `mvnw --version` or `gradlew --version`): 

        Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
        Maven home: /home/kny/.m2/wrapper/dists/apache-maven-3.6.3-bin/1iopthnavndlasol9gbrbg6bf2/apache-maven-3.6.3
        Java version: 11.0.8, vendor: Red Hat, Inc., runtime: /usr/lib/jvm/java-11-openjdk-11.0.8.10-2.fc32.x86_64
        Default locale: en_GB, platform encoding: UTF-8
        OS name: "linux", version: "5.7.15-200.fc32.x86_64", arch: "amd64", family: "unix"


**Additional context**
(Add any other context about the problem here.)
@kny78 kny78 added the kind/bug Something isn't working label Aug 24, 2020
@quarkusbot
Copy link

/cc @evanchooly

@geoand
Copy link
Contributor

geoand commented Aug 24, 2020

Can you please add the output of the failed test run?

@kny78
Copy link
Contributor Author

kny78 commented Aug 24, 2020

@geoand I added the logs to my example git-repo:
https://github.com/kny78/quarkus-kotlin14-jackson-fail/tree/master/failure-logs

@geoand
Copy link
Contributor

geoand commented Aug 25, 2020

@RotBolt do you think this might be similar to the native resources thing you were looking at?

@andreas-eberle
Copy link
Contributor

@kny78: I think you pull some artefacts with the kotlin version 1.3.72 since that is the version in the current quarkus bom. When you do not explicitly specify the 1.4 version for all kotlin artefacts, I think you get mixed versions. Maybe have a look at the dependency tree to verify the versions are all 1.4.

@kny78
Copy link
Contributor Author

kny78 commented Aug 26, 2020

@andreas-eberle I have explicitly added dependencies with 1.4.0 for these.

Please see the output of mvn dependency:tree in:
https://github.com/kny78/quarkus-kotlin14-jackson-fail/blob/master/failure-logs/dependency-tree.txt

@andreas-eberle
Copy link
Contributor

I think jackson-module-kotlin is not Kotlin 1.4 compatible yet: FasterXML/jackson-module-kotlin#356

@RotBolt
Copy link
Contributor

RotBolt commented Aug 26, 2020

@geoand resource classpath is added properly. Probably Resource is not there itself in Kotlin 1.4

Resource works fine on JVM tests. It was the problem with Native resources only.

@kny78
Copy link
Contributor Author

kny78 commented Aug 26, 2020

@RotBolt @geoand I have two identical tests. The only difference is that the one that fails has the @QuarkusTest annotation. The one without works fine.

So I suspect that Quarkus does some classpath manipulations or other stuff that breaks Jackson / Kotlin

@kny78
Copy link
Contributor Author

kny78 commented Aug 26, 2020

@RotBolt @geoand Did any one of you try to clone my github repo, and run the tests your self?

@geoand
Copy link
Contributor

geoand commented Aug 26, 2020

I have not. Hopefully some time next week, currently I don't have enough time

@RotBolt
Copy link
Contributor

RotBolt commented Aug 26, 2020

I will try to run tests on your producer.
Yes Quarkus does add classpath for Jackson. I will take a look then

@geoand
Copy link
Contributor

geoand commented Aug 27, 2020

This doesn't really look like a Quarkus bug... My bet is that it's some obscure Kotlin issue

@kny78
Copy link
Contributor Author

kny78 commented Aug 27, 2020

Yet it only manifests it self if I add the  @QuarkusTest.

Without it, it runs as it should. I can also go up in the stack in the debugger, and it works.

@geoand
Copy link
Contributor

geoand commented Aug 27, 2020

Yeah I saw that, but still, Quarkus isn't coming into play in any way for this test - except for being started.

@kny78
Copy link
Contributor Author

kny78 commented Aug 27, 2020

@geoand Does it change the class path? If so it could be that it adds / does not add certain jars to the classpath on a parent, and that causes the issue?

@geoand
Copy link
Contributor

geoand commented Aug 27, 2020

The classpath is changed for QuarkusTest and that probably does uncovers the issue. But at this point it looks so low level in Kotlin and 1.4 is so new, so I am reluctant to look further into at this stage

@kny78
Copy link
Contributor Author

kny78 commented Aug 27, 2020

I see if I can find the differences in the classpath. Then we might have a startingpoint for either fix Quarkus, og report an upstream bug to the Kotlin project :-)

@pwongv
Copy link

pwongv commented Aug 31, 2020

I appear to have run into a possibly related issue investigating a problem with Kotlin based library (Pact consumer) which also results in java.lang.ExceptionInInitializerError. The problem only manifests when the Quarkus extension is enabled, regardless of whether the test contains anything quarkus specific.

Kotlin's standard library and Pact both use a similar mechanism to determine their versions at runtime:
new JarInputStream(kotlinUnit.getProtectionDomain().getCodeSource().getLocation().openStream());

While debugging, I noticed that .getProtectionDomain().getCodeSource().getLocation() returns a different when @QuarkusTest is enabled or not because QuarkusClassLoader is used.

Without QuarkusTest:
file:/home/username/.m2/repository/au/com/dius/pact/core/model/4.1.7/model-4.1.7.jar

With QuarkusTest
jar:file:/home/username/.m2/repository/au/com/dius/pact/core/model/4.1.7/model-4.1.7.jar!/

This change causes sun.net.www.protocol.jar.JarUrlConnection::getInputStream to get invoked instead which fails on account of a null entry field. However, the javadoc for JarUrlConnection explicitly says "A URL Connection to a Java ARchive (JAR) file or an entry in a JAR file."

Also, JarInputSream expects its input to be a inputstream from a jar rather than a single entry inside the jar.

Edit: Forgot to mention, I've tried this using Quarkus 1.7.0.Final and 1.6.0.Final as well as several versions of Pact Consumer, with no improvement in behavior.

@geoand
Copy link
Contributor

geoand commented Aug 31, 2020

@pwongv that is very useful information, thanks! Do you think you can isolate this into a small code snippet?

That should make easy to reproduce the problem and fix what looks like a bug in the QuarkusClassLoader

@pwongv
Copy link

pwongv commented Aug 31, 2020

Sure thing.
I've spliced the Pact tutorial example into the Quarkus quickstart :

0001-Splicing-in-Pact-test-example-to-demonstrate-11549.patch.TXT

If you set a breakpoint in au.com.dius.pact.core.model.BasePact::lookupVersion you should be able to poke around.

@kny78
Copy link
Contributor Author

kny78 commented Sep 6, 2020

Does anyone know the status on this? Is the problem reported to the people who work with the classloader?

@geoand
Copy link
Contributor

geoand commented Sep 6, 2020

I would be one of those people but unfortunately I haven't had time to look into it yet.

@kny78
Copy link
Contributor Author

kny78 commented Sep 6, 2020

I know the feeling :-)

@geoand
Copy link
Contributor

geoand commented Sep 6, 2020

Hopefully sometime this week 😎

@geoand
Copy link
Contributor

geoand commented Sep 10, 2020

Looks like fun :)

@geoand
Copy link
Contributor

geoand commented Sep 10, 2020

Took a super quick look at this and didn't get far... If someone with knowledge of Kotlin internals want to take a stab at this, be my guest :)

Or if someone that at least knows where Kotlin Rrflect does any sort of ClassLoading and provide those hints I can take another look.

@pschyma
Copy link

pschyma commented Sep 10, 2020

I tried to debug this for a short time, but my breakpoints were never hit in the tests.

Maybe I'll find some time on the weekend to understand kotlin internals here.

Meanwhile I made another observation: when I remove the "reflect" dependency and Gradle pulls the 1.3.72 version from jackson-module-kotlin, my tests work again on Quarkus 1.8.0.Final. On Quarkus 1.7.3.Final hibernate-types fails to deserialize a JSON field because it gets confused by the class hierarchy of that field (serialized as @JsonTypeInfo(use = Id.MINIMAL_CLASS, include = As.WRAPPER_OBJECT)).

I'm not sure who to blame now: Quarkus, Kotlin, Jackson, ... so I'll go back to kotlin 1.3.72 and wait for the jackson module update.

@kny78
Copy link
Contributor Author

kny78 commented Sep 16, 2020

Did anyone have any time to dig any further into this?

@pschyma
Copy link

pschyma commented Sep 16, 2020

Unfortunately not enough to get an deep understanding of the things happening there :-/.

@kny78
Copy link
Contributor Author

kny78 commented Oct 13, 2020

Tried with jackson 2.11.3, but it still fails.

@kny78
Copy link
Contributor Author

kny78 commented Oct 28, 2020

I have tried Quarkus 1.9.0, and it also includes Jackson 2.11.3, but still get the same error.
https://github.com/kny78/quarkus-kotlin14-jackson-fail/

@geoand Did you have any time to look into this?

@geoand
Copy link
Contributor

geoand commented Oct 28, 2020

Unfortunately no. My time is on very short supply lately...

@pflorek
Copy link

pflorek commented Nov 4, 2020

I have also having Jackson 2.11.3 with Quarkus 1.9.1 and Kotlin 1.4.10

JUnit tests are green. Running simple fun main() from IntelliJ works. Downgrading Kotlin to 1.3.72 works. But starting quarkusDev results in Caused by: java.lang.AssertionError: Built-in class kotlin.Any is not found

The reason for me jackson-module-kotlin depends on Kotlin 1.3.72

Currently 2.12 for Kotlin 1.4.10 is not yet released 😢

Maybe that's the problem...

@kny78
Copy link
Contributor Author

kny78 commented Nov 5, 2020

@pflorek I have tested with 2.12.0-rc1, and the same issue appeared. I also tried to upgrade Kotlin to 1.4.20-RC, but to no avail.

Also the same code runs without @QuarkusTest , but not with it.

See:
Fail: https://github.com/kny78/quarkus-kotlin14-jackson-fail/blob/master/src/test/kotlin/im/kny/DeSerializeQuarkusTest.kt
Works: https://github.com/kny78/quarkus-kotlin14-jackson-fail/blob/master/src/test/kotlin/im/kny/DeSerializeNonQuarkusTest.kt

@hrensgory
Copy link

Filed issue to Kotlin team, perhaps they can shed some light on it: https://youtrack.jetbrains.com/issue/KT-43438

@kny78
Copy link
Contributor Author

kny78 commented Nov 18, 2020

@hrensgory @geoand It has already been reported to Kotlin, but they have said:

Thanks for the report. It looks like the issue was in the Quarkus class loader, so I'll close this one.

And if we remove @QuarkusTest, then it runs as it should.

Is it possible to easily check if the classloader that calls the test-code has kotlin-stdlib on the classpath?

@kny78
Copy link
Contributor Author

kny78 commented Nov 18, 2020

Did anyone clone the test-repo, and try to run the code? It is updated with Quarkus 1.9.2!

@hrensgory
Copy link

hrensgory commented Nov 19, 2020

@hrensgory @geoand It has already been reported to Kotlin, but they have said:

Thanks for the report. It looks like the issue was in the Quarkus class loader, so I'll close this one.

@kny78 - as for now bug isn't declined, so vote for it ;-) !

And if we remove @QuarkusTest, then it runs as it should.
Is it possible to easily check if the classloader that calls the test-code has kotlin-stdlib on the classpath?

I added quarkus-bootstrap-maven plugin and called quarkus-bootstrap-maven-plugin:dev-mode-tree as well as quarkus-bootstrap-maven-plugin:build-tree but I didn't find anything suspicious in the output. Could you check it as well, please, I can miss something.

@pschyma
Copy link

pschyma commented Nov 19, 2020

It's getting a bit urgent now: with 1.10.CR1 Kotlin 1.4.10 is pulled into the dependencies when one uses testImplementation("io.rest-assured:kotlin-extensions:4.3.1") and does not enforce the Quarkus platform (implementation(platform("$quarkusPlatformGroupId:$quarkusPlatformArtifactId:$quarkusPlatformVersion"))).

With my limited time right now and my lack of understanding the Quarkus class loading, my efforts have not shown much success for now.

@evanchooly
Copy link
Member

I'm actively looking in to this for what it's worth but it's leading in to parts of quarkus I'm not super familiar with so it's taking time.

@kny78
Copy link
Contributor Author

kny78 commented Nov 20, 2020

Maybe a stupid, and out of place question, but does Quarkus / Jboss really need a special ClassLoader for Quarkus? Quarkus is meant for microservices, and not as a "Application Server".

Is it possible to disable the custom ClassLoader?

@geoand
Copy link
Contributor

geoand commented Nov 20, 2020

In test mode and dev mode, the special ClassLoader is absolutely necessary

@kny78
Copy link
Contributor Author

kny78 commented Nov 23, 2020

@geoand @evanchooly I see that in IntelliJ, it adds a special "Quarkus Deployment" under dependencies. Here these libs are added:

  • kotlin-compiler-1.3.72.jar
  • kotlin-reflect-1.3.72.jar
  • kotlin-script-runtime-1.3.72.jar
  • kotlin-stdlib-1.3.72.jar
  • kotlin-stdlib-common-1.3.72.jar

Is the quarkus-maven-plugin or some other code modifying the dependencies?

@udalov
Copy link

udalov commented Nov 23, 2020

Hi! Here's a bit of info from Kotlin's point of view. I'm not sure how much of this is useful but it might help in tracking down the issue.

The main relevant change in Kotlin from 1.3.72 to 1.4.0 was that now we're bundling Java module system's module-info files into stdlib, reflect and other libraries (KT-21266). Prior to 1.4.0, if main Kotlin libraries were used in the modular world, i.e. in the Java module path, they were loaded as automatic modules.

So starting from 1.4.0, kotlin-stdlib and kotlin-reflect now have META-INF/versions/9/module-info.class, which works the following way as per JEP 238 "multi-release jars": on JDKs < 9 it's treated as an irrelevant resource and effectively ignored, and on JDKs >= 9 it's loaded as if it was located in the root, and so is treated as the definition of the corresponding module.

Here are the module-info definitions of: kotlin-stdlib, kotlin-reflect. The main parts here are that kotlin-reflect requires kotlin-stdlib, and that kotlin-stdlib opens a bunch of packages to kotlin-reflect, which allows the latter to load resources located in stdlib.

The error you're seeing is saying that kotlin-reflect tries to load a resource and fails. The resource in question is kotlin/kotlin.kotlin_builtins which is located in kotlin-stdlib. Since class/resource loading behavior changed with the introduction of Java module system, there's also different code handling this in kotlin-reflect, employing the same multi-release jar feature mentioned above.

For JDKs < 9, we invoke ClassLoader.getResourceAsStream on the class loader of kotlin-reflect, falling back to the system class loader if needed. It's assumed that the class loader which loaded kotlin-reflect is also able to load kotlin-stdlib, where the needed resource is located, otherwise nothing would work at all. For JDKs >= 9, we invoke Module.getResourceAsStream on the kotlin-stdlib module. This works because kotlin-stdlib opens the needed package to kotlin-reflect. We locate the stdlib module by getting it out from an arbitrary stdlib class, in this case kotlin.Unit.

So it might be the case that the special class loader you're using in Quarkus is somehow not creating the module structure correctly, which either leads to kotlin.Unit having no module or incorrect module, or Module.getResourceAsStream behaving incorrectly.

One other major note: we had a bug in 1.4.0 where kotlin-reflect did not work at all if it was used in the Java module path with the standard Java module system's class loader (KT-40842). It was fixed in Kotlin 1.4.10. It shouldn't necessarily affect this issue, but it makes sense to test only 1.4.10 just in case.

@stuartwdouglas
Copy link
Member

There is a really simple fix here to just make Kotlin be parent first, so it is loaded from the system CL rather than the QuarkusClassLoader.

I have not been able to figure out exactly what is going on here though. As far as I can tell QuarkusClassLoader.getResourceAsStream is not actually invoked.

stuartwdouglas added a commit to stuartwdouglas/quarkus that referenced this issue Nov 24, 2020
Also change to 1.4

Fixes quarkusio#11549
stuartwdouglas added a commit to stuartwdouglas/quarkus that referenced this issue Nov 24, 2020
Also change to 1.4

Fixes quarkusio#11549
@evanchooly
Copy link
Member

I came to the same conclusion yesterday as Stuart and he has a fix/PR in place that shows it working. There's a tweak or two to make on it but i'm running those final tests now. Thanks for the explainer @udalov. I've spent a fair bit of time over the last few days in that code and it's ... dense. :)

stuartwdouglas added a commit to stuartwdouglas/quarkus that referenced this issue Nov 25, 2020
Also change to 1.4

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