grpc / grpc-kotlin

Kotlin gRPC implementation. HTTP/2 based RPC
https://grpc.io/docs/languages/kotlin
Apache License 2.0
1.2k stars 166 forks source link

Kotlin Coroutines server impl: unable to use aspects due to CGLIB and final "context" property #261

Closed Viachaslau-Zinkevich closed 3 years ago

Viachaslau-Zinkevich commented 3 years ago

For Kotlin Coroutines service/server implementations generated code uses io.grpc.kotlin.AbstractCoroutineServerImpl. As GRPC generator doesn't create any service interfaces, Spring's AutoProxy aspect weaver cannot use JDK proxies and falls back to CGLIB. io.grpc.kotlin.AbstractCoroutineServerImpl uses val context: CoroutineContext which is a non-open non-overridable property which generates final getter. This results in a situation when CGLIB must generate a new field and doesn't delegate getter call to original proxied target, thus having context set to null. Having context = null fails generated *CoroutineImplBase#bindService implementations, which expects this.couroutine to be non-null.

Making context as open val context in io.grpc.kotlin.AbstractCoroutineServerImpl will potentially fix the issue as it will allow override and extensions for CGLIB.

In the long run ideally I believe the generated service must have interfaces, to avoid such problems in future and do not depend on abstract classes.

bjoernmayer commented 3 years ago

Adding to this: As soon as we encounter an issue in a context it cannot be reused for further request. In Kroto+ we fixed this by creating the context, everytime it is used:

override val initialContext: CoroutineContext
        get() = taskExecutor.toNewCoroutineScope().coroutineContext

        // toNewCoroutineScope is our own extension function       

Here in Grpc-Kotlin we cannot do this since the val is final.

I suggest to open it and to move its definition outside of the constructor.

abstract class AbstractCoroutineServerImpl(
  /** The context in which to run server coroutines. */
  context: CoroutineContext = EmptyCoroutineContext
) : BindableService {
    open val context: CoroutineContext = context
}