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

Kotlin Coroutines Breaks Jackson Serialization #7999

Closed
sherl0cks opened this issue Mar 19, 2020 · 16 comments
Closed

Kotlin Coroutines Breaks Jackson Serialization #7999

sherl0cks opened this issue Mar 19, 2020 · 16 comments
Labels
area/kotlin kind/bug Something isn't working triage/out-of-date This issue/PR is no longer valid or relevant

Comments

@sherl0cks
Copy link
Contributor

sherl0cks commented Mar 19, 2020

Describe the bug
See https://github.com/sherl0cks/kotlin-coroutines-quarkus-graal-jackson-reproducer/blob/master/README.md

It is possible that this should be opened up in https://github.com/oracle/graal. If so, I would appreciate some assistance on that from the Quarkus team as you all have been very responsive and probably have better contacts with Graal than I do.

Expected behavior
Jackson works in coroutines like it does without coroutines.

Actual behavior
Some variation of the below exception depending on the versions of libraries:

2020-03-19 15:02:43,528 ERROR [io.qua.ver.htt.run.QuarkusErrorHandler] (executor-thread-1) HTTP Request to /greeting/normal failed, error id: 68a91fde-fc23-44cd-a4e2-3d9735b85543-1: org.jboss.resteasy.spi.UnhandledException: java.lang.IllegalStateException: @NotNull method kotlin/reflect/jvm/internal/impl/builtins/KotlinBuiltIns.getBuiltInClassByFqName must not return null

To Reproduce
See https://github.com/sherl0cks/kotlin-coroutines-quarkus-graal-jackson-reproducer/

Environment (please complete the following information):
See https://github.com/sherl0cks/kotlin-coroutines-quarkus-graal-jackson-reproducer/

Additional context
Kotlin coroutines and graal already have known issues: oracle/graal#366

@sherl0cks sherl0cks added the kind/bug Something isn't working label Mar 19, 2020
@sherl0cks
Copy link
Contributor Author

@geoand ☝️

@sherl0cks sherl0cks changed the title Kotlin Coroutines Kotlin Coroutines Breaks Jackson Serialization Mar 19, 2020
@geoand
Copy link
Contributor

geoand commented Mar 19, 2020

Thanks for reporting @sherl0cks.

IIUC, co-routines don't work at all on native-image, right?

If that is the case, I don't know what we can do.
Perhaps @dmlloyd has more insight

@sherl0cks
Copy link
Contributor Author

sherl0cks commented Mar 19, 2020

@geoand they work, but there are issues for sure. I think from a Quarkus perspective, there needs to be clear guidance to Kotlin users how to do nonblocking and async. If not coroutines, then what? Should users incorporate vert.x? Stick to completeable future without the coroutines bridge? IMO - adding this to the Kotlin guide with a strong opinion is important. Otherwise users like my team will fall into the void.

That said, when coroutines work, they are great! So it would nice to also put some pressure on oracle from the quarkus side to get it working well (see the comments in the bottom of oracle/graal#366).

@geoand
Copy link
Contributor

geoand commented Mar 19, 2020

Thanks for the input @sherl0cks.

cc @emmanuelbernard @n1hility for input on how we should frame our suggestions in the guide

@geoand
Copy link
Contributor

geoand commented Mar 19, 2020

I'll just add that out general reactive strategy is focused on Vert.x and Mutiny

@sherl0cks
Copy link
Contributor Author

@emmanuelbernard @n1hility bumping this. It would be really nice to have clear guidance on the kotlin guide here. I'm happy to submit a PR, but I don't know how you want to frame this. Also worth noting that there is now progress with Graal and kotlin coroutines which should be released in Graal 20.1 oracle/graal#1330.

The other item that came to mind is the way the AWS lambda integration is written, and my understanding is that it is blocking by design (e.g. cannot handle multiple lambda events at once, unlike the native Node.js lambda runtime). So @patriot1burke may have input there.

@emmanuelbernard
Copy link
Member

On the co-routine aspect I would frame it as such. GraalVM and Kotlin co-routines are work in progress blah blah. You can use them in Quarkus apps but native executable support is hit and miss. The Quarkus way to do reactive is via Mutiny and vert.x as it brings more to the table than co-routines. See https://blog.softwaremill.com/will-project-loom-obliterate-java-futures-fb1a28508232 for a great in depth article.
(please rephrase but that's the gist)

Whether we want to bring some co-routine friendly APIs to Mutiny (if that is even a thing), I'd defer to @evanchooly (with @cescoffier guidance).

@wowselim
Copy link
Contributor

wowselim commented Apr 8, 2020

Whether we want to bring some co-routine friendly APIs to Mutiny...

Vert.x actually has pretty decent support for Kotlin's co-routines already and I hope it works well on GraalVM in the future. 🤞
This way kotlin flow could also provide an alternative to mutiny

@turbolaban
Copy link

Stumbled upon this issue as I ran into a similar problem. After a bit of testing, it appears that it is not related to coroutines at all, but rather that the Jackson Kotlin module is missing or broken in the native build. If the code can be made to run without the jackson kotlin module, it works both on jvm and native. With and without coroutines.

A simple example, without coroutines, that works on jvm and fails native:

import javax.ws.rs.Consumes
import javax.ws.rs.POST
import javax.ws.rs.Path
import javax.ws.rs.Produces
import javax.ws.rs.core.MediaType

data class Person(
    val name: String,
    val address: Address
)

data class Address(
    val street: String
)

@Path("/hello")
class ExampleResource {

    @POST
    @Consumes(MediaType.APPLICATION_JSON)
    @Produces(MediaType.APPLICATION_JSON)
    fun hello(person: Person) = person
}

The error message presented when trying the native build, is the same as one would get on jvm if the kotlin module for jackson is removed.

The above code can be made to run without the kotlin module by changing the Address class to

data class Address(
    @JsonProperty("street")
    val street: String
)

In which case the native build also works.

If the code is wrapped in a coroutine, the actual error is hidden by the kotlin reflect stuff.

Hopefully you are able to get the jackson kotlin module working also in a native build :)

@heubeck
Copy link
Contributor

heubeck commented Nov 7, 2021

Still experiencing this with Quarkus 2.4.1, Kotlin 1.5.31 using compiler target 16 and Mandrel 21.3-java17

@heubeck
Copy link
Contributor

heubeck commented Nov 12, 2021

Don't know if it's directly related to this ticket, but maybe helps:
Even without coroutines, my Kotlin project doesn't work in native mode, when build with the default build container quay.io/quarkus/ubi-quarkus-native-image, tested with 21.1-java11, 21.2-java11, 21.3-java11 and 21.3-java17.

Native app starts, but on first operation it fails with the beloved KotlinBuiltIns.getBuiltInClassByFqName must not return null.
Could not made it work with all the tips given here.

BUT: Building with the quay.io/quarkus/ubi-quarkus-native-image containers, it works well and great and without errors and is really awesome.

@geoand
Copy link
Contributor

geoand commented Jan 11, 2022

@heubeck I am not sure I understand under which cases it works and which it doesn't

@heubeck
Copy link
Contributor

heubeck commented Jan 11, 2022

Hey @geoand

pardon me, I've obviously named the wrong containers and can't reproduce it right now.

At a certain point in time (around begin of November 2021), the container based native image build produced binaries, that failed with the error shown above.
Solution was to configure the explicit builder image
<quarkus.native.builder-image>quay.io/quarkus/ubi-quarkus-native-image:21.3-java17</quarkus.native.builder-image>
as it seems, that the default builder-image was a different one (at this time).

Trying it now makes no difference, also without having an builder image configured, it defaults to ubi-quarkus-native-image.

Pardon me for the confusion, I assume there's no issue anymore (or never has been, and it was just me).

Was there a change of the default builder image used?

@geoand
Copy link
Contributor

geoand commented Jan 11, 2022

No problem at all.

The name of the image itself has not changed, but the image itself has. See https://github.com/quarkusio/quarkus-images/commits/main/quarkus-native-image.yaml for more details

@evanchooly
Copy link
Member

It looks like this one can be closed then?

@geoand
Copy link
Contributor

geoand commented Feb 2, 2022

Let's close for now.

If the issue persists, comments are welcome

@geoand geoand closed this as completed Feb 2, 2022
Kotlin roadmap automation moved this from Todo to Done Feb 2, 2022
@geoand geoand added the triage/out-of-date This issue/PR is no longer valid or relevant label Feb 2, 2022
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 triage/out-of-date This issue/PR is no longer valid or relevant
Projects
Development

No branches or pull requests

8 participants