Quarkus Cache always executes or deadlocks on Kotlin coroutines

[kdubb]

Describe the bug

When using @CacheResult on a Kotlin coroutine method the method is either always called (i.e. nothing is cached) or the method deadlocks.

No Caching

The following example method, the method is always called; no caching is done.

suspend getData(id: String): List<Data> {...}

This appears to be because the kotlin.coroutines.Continuation completion parameter is included in the cache key.

Deadlock

You can "fix" the inclusion of the completion parameter using the @CacheKey annotation. Unfortunately, this causes a deadlock.

Adding the annotation to the original example, causes any call to the cached method to deadlock.

suspend getData(@CacheKey id: String): List<Data> {...}

When calling the method the invocation happens twice, and the second time deadlocks waiting for the cached value to be fulfilled.

Expected behavior

Kotlin coroutine methods work the same way as synchronous or reactive methods.

Kotlin continuation parameters (kotlin.coroutines.Continuation) should always be excluded from the cache key with having to use the @CacheKey annotation.

Actual behavior

Either no caching happen or the thread deadlocks when calling a coroutine with the @CacheResult.

How to Reproduce?

1. Run the attached attached demo using ./mvnw quarkus:dev.
2. Calling https://localhost:8080/hello/test1 with return a new timestamp when it should be cached indefinitely after the first invocation.
3. Calling https://localhost:8080/hello/test2 will deadlock the request (and apparently the CLI along with it).

code-with-quarkus.zip

Output of uname -a or ver

macOS 12

Output of java -version

Java version "17" 2021-09-14 LTS Java(TM) SE Runtime Environment (build 17+35-LTS-2724) Java HotSpot(TM) 64-Bit Server VM (build 17+35-LTS-2724, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.4.2 & 2.5.0

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

[quarkus-bot]

/cc @evanchooly, @gwenneg

[gwenneg]

Thanks for reporting this @kdubb.

I called http://localhost:8080/hello/test1 with:

• quarkus.log.category."io.quarkus.cache".level=DEBUG in the application.properties file
• a CompositeCacheKey#toString implementation that shows the details of the composite key

Here's the result in the Quarkus log:

2021-11-21 22:22:12,698 DEBUG [io.qua.cac.run.CacheResultInterceptor] (vert.x-eventloop-thread-3) Loading entry with key [[a, Continuation at org.jboss.resteasy.reactive.server.runtime.kotlin.CoroutineInvocationHandler$handle$1.invokeSuspend(CoroutineInvocationHandler.kt:40)]] from cache [test1]
2021-11-21 22:22:12,712 DEBUG [io.qua.cac.run.CacheResultInterceptor] (vert.x-eventloop-thread-3) Loading entry with key [[org.acme.DataProvider_Subclass@311f2dd6, a, Continuation at org.jboss.resteasy.reactive.server.runtime.kotlin.CoroutineInvocationHandler$handle$1.invokeSuspend(CoroutineInvocationHandler.kt:40)]] from cache [test1]

@mkouba Do you know why an interceptor would be invoked by ArC twice with varying arguments when the intercepted method is a Kotlin coroutine?

[geoand]

Kotlin continuation parameters (kotlin.coroutines.Continuation) should always be excluded from the cache key with having to use the @Cachekey annotation.

This is definitely part of the issue and we should take care of it

[gwenneg]

This is definitely part of the issue and we should take care of it

Yes, this should be an easy fix. But first I'd like to understand why the intercepted method is invoked twice.

[kdubb]

@geoand @gwenneg I actually readied a PR for excluding the continuation, but since it just causes a deadlock by default due to the doubling issue, it's not all that helpful at the moment.

[gsmet]

Let's revive this one. /cc @mkouba , see @gwenneg 's question above ^

[mkouba]

This is definitely part of the issue and we should take care of it

Yes, this should be an easy fix. But first I'd like to understand why the intercepted method is invoked twice.

I have no idea. But I can try to take a look at the reproducer...

[mkouba]

So it seems that DataProvider.class has a synthetic static method for each coroutine, e.g. get1$suspendImpl(org.acme.DataProvider, java.lang.String, kotlin.coroutines.Continuation) and this method has the same annotations like the original method, i.e. get1(java.lang.String, kotlin.coroutines.Continuation<? super java.util.List<java.lang.String>>). The cache interceptor is first invoked for the coroutine and then for the static method.

I think that we can ignore synthetic static methods with interceptor bindings. On the other hand, I don't think that we ignore non-static synthetic methods with interceptor bindings. @manovotn WDYT?

[mkouba]

For the record - I assume that the only reason why we don't ignore the non-static synthetic methods is that we expect that a framework can add a synthetic method that needs to be intercepted. But I'm not really sure this is something that should be supported.

[manovotn]

For the record - I assume that the only reason why we don't ignore the non-static synthetic methods is that we expect that a framework can add a synthetic method that needs to be intercepted. But I'm not really sure this is something that should be supported.

You beat me to it. I was going to ask if this is something we want to support anyway. It might even be surprising that such method gets intercepted. Personally, I think we could ignore both cases but at least synthetic static methods can definitely be skipped.

[mkouba]

@gwenneg @manovotn PR sent: #22586 ;-)

[geoand]

Closing this in favor of #23746