search

Kotlin / kotlinx.coroutines

Library support for Kotlin coroutines
Apache License 2.0
13.04k stars 1.85k forks source link

BlockHound false positive in kotlin.jvm.internal.Reflection.renderLambdaToString #4128

Open OliverO2 opened 5 months ago

OliverO2 commented 5 months ago

kotlinx-coroutines-debug:1.8.0

This one appeared once when running Kotest in debug mode with env KOTEST_DEBUG=true ./gradlew ...:

reactor.blockhound.BlockingOperationError: Blocking call! jdk.internal.misc.Unsafe#park
    at app//io.kotest.extensions.blockhound.KotestBlockHoundIntegration.applyTo$lambda$2$lambda$1(KotestBlockHoundIntegration.kt:28)
    at app//reactor.blockhound.BlockHound$Builder.lambda$install$8(BlockHound.java:488)
    at reactor.blockhound.BlockHoundRuntime.checkBlocking(BlockHoundRuntime.java:89)
    at java.base@17.0.10/jdk.internal.misc.Unsafe.park(Unsafe.java)
    at java.base@17.0.10/java.util.concurrent.locks.LockSupport.park(LockSupport.java:211)
    at java.base@17.0.10/java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(AbstractQueuedSynchronizer.java:715)
    at java.base@17.0.10/java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(AbstractQueuedSynchronizer.java:938)
    at java.base@17.0.10/java.util.concurrent.locks.ReentrantLock$Sync.lock(ReentrantLock.java:153)
    at java.base@17.0.10/java.util.concurrent.locks.ReentrantLock.lock(ReentrantLock.java:322)
    at app//kotlin.reflect.jvm.internal.impl.storage.DefaultSimpleLock.lock(locks.kt:48)
    at app//kotlin.reflect.jvm.internal.impl.storage.LockBasedStorageManager$MapBasedMemoizedFunction.invoke(LockBasedStorageManager.java:554)
    at app//kotlin.reflect.jvm.internal.impl.serialization.deserialization.TypeDeserializer.typeConstructor(TypeDeserializer.kt:156)
    at app//kotlin.reflect.jvm.internal.impl.serialization.deserialization.TypeDeserializer.simpleType(TypeDeserializer.kt:91)
    at app//kotlin.reflect.jvm.internal.impl.serialization.deserialization.TypeDeserializer.type(TypeDeserializer.kt:68)
    at app//kotlin.reflect.jvm.internal.impl.serialization.deserialization.MemberDeserializer.loadFunction(MemberDeserializer.kt:218)
    at app//kotlin.reflect.jvm.ReflectLambdaKt$reflect$descriptor$1.invoke(reflectLambda.kt:46)
    at app//kotlin.reflect.jvm.ReflectLambdaKt$reflect$descriptor$1.invoke(reflectLambda.kt:46)
    at app//kotlin.reflect.jvm.internal.UtilKt.deserializeToDescriptor(util.kt:267)
    at app//kotlin.reflect.jvm.ReflectLambdaKt.reflect(reflectLambda.kt:45)
    at app//kotlin.reflect.jvm.internal.ReflectionFactoryImpl.renderLambdaToString(ReflectionFactoryImpl.java:56)
    at app//kotlin.reflect.jvm.internal.ReflectionFactoryImpl.renderLambdaToString(ReflectionFactoryImpl.java:51)
    at app//kotlin.jvm.internal.Reflection.renderLambdaToString(Reflection.java:79)
    at app//kotlin.jvm.internal.Lambda.toString(Lambda.kt:11)
    at java.base@17.0.10/java.lang.String.valueOf(String.java:4220)
    at java.base@17.0.10/java.lang.StringBuilder.append(StringBuilder.java:173)
    at app//io.kotest.core.test.config.ResolvedTestConfig.toString(ResolvedTestConfig.kt)
    at java.base@17.0.10/java.lang.String.valueOf(String.java:4220)
    at java.base@17.0.10/java.lang.StringBuilder.append(StringBuilder.java:173)
    at app//io.kotest.core.test.TestCase.toString(TestCase.kt)
    at java.base@17.0.10/java.lang.String.valueOf(String.java:4220)
    at java.base@17.0.10/java.lang.StringBuilder.append(StringBuilder.java:173)
    at app//io.kotest.core.test.TestCase.toString(TestCase.kt)
    at java.base@17.0.10/java.lang.String.valueOf(String.java:4220)
    at java.base@17.0.10/java.lang.StringBuilder.append(StringBuilder.java:173)
    at app//io.kotest.engine.test.listener.TestCaseExecutionListenerToTestEngineListenerAdapter$testFinished$2.invoke(TestCaseExecutionListenerToTestEngineListenerAdapter.kt:19)
    at app//io.kotest.engine.test.listener.TestCaseExecutionListenerToTestEngineListenerAdapter$testFinished$2.invoke(TestCaseExecutionListenerToTestEngineListenerAdapter.kt:19)
    at app//io.kotest.mpp.Logger$log$1.invoke(logger.kt:19)
    at app//io.kotest.mpp.Logger$log$1.invoke(logger.kt:18)
    at app//io.kotest.mpp.LoggerKt.log(logger.kt:41)
    at app//io.kotest.mpp.Logger.log(logger.kt:18)
    at app//io.kotest.engine.test.listener.TestCaseExecutionListenerToTestEngineListenerAdapter.testFinished(TestCaseExecutionListenerToTestEngineListenerAdapter.kt:19)
    at app//io.kotest.engine.test.interceptors.TestFinishedInterceptor.intercept(TestFinishedInterceptor.kt:26)
    at app//io.kotest.engine.test.interceptors.TestFinishedInterceptor$intercept$1.invokeSuspend(TestFinishedInterceptor.kt)
    at app//kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
    at app//kotlinx.coroutines.internal.ScopeCoroutine.afterResume(Scopes.kt:28)
    at app//kotlinx.coroutines.AbstractCoroutine.resumeWith(AbstractCoroutine.kt:99)
    at app//kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:46)
    at app//kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
    at app//kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:585)
    at app//kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:802)
    at app//kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:706)
    at app//kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:693)

Might be worth adding an allowance to BlockHound.Builder.allowBlockingCallsInReflectionImpl() at whatever level is deemed appropriate.

qwwdfsad commented 5 months ago

The error complains about our reflect library and we (coroutines) are unlikely to ship BH rules for anything but coroutines themselves.

Would it work for you to add a rule in your project for the whole Kotlin reflect package? On a side note, the fact that renderToString acquires a global lock is issue on its own, but alas it's more complicated then "just fix it"

dkhalanskyjb commented 5 months ago

we (coroutines) are unlikely to ship BH rules for anything but coroutines themselves.

We already do: https://github.com/Kotlin/kotlinx.coroutines/blob/cd696d3f8f4afdbc735915c2bded974616331b55/kotlinx-coroutines-debug/src/CoroutinesBlockHoundIntegration.kt#L173-L183

for the whole Kotlin reflect package?

A bit scary, given that reflection can execute arbitrary user-supplied code.

OliverO2 commented 5 months ago

The error complains about our reflect library and we (coroutines) are unlikely to ship BH rules for anything but coroutines themselves.

Surely a balancing issue, just thought to let you know because some reflection allowance was already there.

Would it work for you to add a rule in your project for the whole Kotlin reflect package?

Sure, no big deal. [Edit: Not the whole package, but selected reflection functions.]

On a side note, the fact that renderToString acquires a global lock is issue on its own, but alas it's more complicated then "just fix it"

Agreed.