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

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

Open
OliverO2 opened this issue May 13, 2024 · 3 comments
Labels

Comments

@OliverO2
Copy link
Contributor

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
Copy link
Member

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
Copy link
Collaborator

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

We already do:

allowBlockingCallsInside("kotlin.reflect.jvm.internal.impl.resolve.OverridingUtil", "<clinit>")
}
/**
* Allows some blocking calls from the reflection API.
*
* The API is big, so surely some other blocking calls will show up, but with these rules in place, at least some
* simple examples work without problems.
*/
private fun BlockHound.Builder.allowBlockingCallsInReflectionImpl() {
allowBlockingCallsInside("kotlin.reflect.jvm.internal.impl.builtins.jvm.JvmBuiltInsPackageFragmentProvider", "findPackage")

for the whole Kotlin reflect package?

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

@OliverO2
Copy link
Contributor Author

OliverO2 commented May 16, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants