From b03e04232e7c3f564f349324aa856f1cd89f92f3 Mon Sep 17 00:00:00 2001 From: Evgeniy Moiseenko Date: Tue, 18 Feb 2025 21:40:49 +0100 Subject: [PATCH] Fix instrumentation of newly loaded classes (#525) --- .../strategy/managed/ObjectLabelFactory.kt | 19 +++++++++++-- .../transformation/LincheckJavaAgent.kt | 27 ++++++++++++------ .../RunConcurrentRepresentationTests.kt | 27 ++++++++++++++++++ .../run_concurrent_test/anonymous_object.txt | 28 +++++++++++++++++++ .../anonymous_object_jdk8.txt | 28 +++++++++++++++++++ 5 files changed, 117 insertions(+), 12 deletions(-) create mode 100644 src/jvm/test/resources/expected_logs/run_concurrent_test/anonymous_object.txt create mode 100644 src/jvm/test/resources/expected_logs/run_concurrent_test/anonymous_object_jdk8.txt diff --git a/src/jvm/main/org/jetbrains/kotlinx/lincheck/strategy/managed/ObjectLabelFactory.kt b/src/jvm/main/org/jetbrains/kotlinx/lincheck/strategy/managed/ObjectLabelFactory.kt index 262666144f..e431a5a75d 100644 --- a/src/jvm/main/org/jetbrains/kotlinx/lincheck/strategy/managed/ObjectLabelFactory.kt +++ b/src/jvm/main/org/jetbrains/kotlinx/lincheck/strategy/managed/ObjectLabelFactory.kt @@ -70,10 +70,23 @@ object ObjectLabelFactory { if (obj is Thread) { return "Thread#${getObjectNumber(Thread::class.java, obj)}" } - if (obj.javaClass.isAnonymousClass) { - return obj.javaClass.simpleNameForAnonymous + runCatching { + if (obj.javaClass.isAnonymousClass) { + return obj.javaClass.simpleNameForAnonymous + } } - return objectName(obj) + "#" + getObjectNumber(obj.javaClass, obj) + val objectName = runCatching { + objectName(obj) + "#" + getObjectNumber(obj.javaClass, obj) + } + // There is a Kotlin compiler bug that leads to exception + // `java.lang.InternalError: Malformed class name` + // when trying to query for a class name of an anonymous class on JDK 8: + // - https://youtrack.jetbrains.com/issue/KT-16727/ + // in such a case we fall back to returning `` class name. + .getOrElse { + "" + } + return objectName } private fun objectName(obj: Any): String { diff --git a/src/jvm/main/org/jetbrains/kotlinx/lincheck/transformation/LincheckJavaAgent.kt b/src/jvm/main/org/jetbrains/kotlinx/lincheck/transformation/LincheckJavaAgent.kt index 6f89e76884..2fc4e5483e 100644 --- a/src/jvm/main/org/jetbrains/kotlinx/lincheck/transformation/LincheckJavaAgent.kt +++ b/src/jvm/main/org/jetbrains/kotlinx/lincheck/transformation/LincheckJavaAgent.kt @@ -133,8 +133,11 @@ internal object LincheckJavaAgent { // old classes that were already loaded before and have coroutine method calls inside canonicalClassName in coroutineCallingClasses } - instrumentation.retransformClasses(*classes.toTypedArray()) - instrumentedClasses.addAll(classes.map { it.name }) + // for some reason, without `isNotEmpty()` check this code can throw NPE on JVM 8 + if (classes.isNotEmpty()) { + instrumentation.retransformClasses(*classes.toTypedArray()) + instrumentedClasses.addAll(classes.map { it.name }) + } } // In the model checking mode, Lincheck processes classes lazily, only when they are used. @@ -192,8 +195,11 @@ internal object LincheckJavaAgent { canonicalClassName in instrumentedClasses } // `retransformClasses` uses initial (loaded in VM from disk) class bytecode and reapplies - // transformations of all agents that did not remove their transformers to this moment - instrumentation.retransformClasses(*classes.toTypedArray()) + // transformations of all agents that did not remove their transformers to this moment; + // for some reason, without `isNotEmpty()` check this code can throw NPE on JVM 8 + if (classes.isNotEmpty()) { + instrumentation.retransformClasses(*classes.toTypedArray()) + } // Clear the set of instrumented classes. instrumentedClasses.clear() } @@ -351,12 +357,15 @@ internal object LincheckClassFileTransformer : ClassFileTransformer { protectionDomain: ProtectionDomain?, classBytes: ByteArray ): ByteArray? = runInIgnoredSection { - if (classBeingRedefined == null) { - // No internal class name is expected if no class is provided. - return null - } else { - require(internalClassName != null) { "Class name must not be null" } + if (classBeingRedefined != null) { + require(internalClassName != null) { + "Internal class name of redefined class ${classBeingRedefined.name} must not be null" + } } + // Internal class name could be `null` in some cases (can be witnessed on JDK-8), + // this can be related to the Kotlin compiler bug: + // - https://youtrack.jetbrains.com/issue/KT-16727/ + if (internalClassName == null) return null // If the class should not be transformed, return immediately. if (!shouldTransform(internalClassName.canonicalClassName, instrumentationMode)) { return null diff --git a/src/jvm/test/org/jetbrains/kotlinx/lincheck_test/representation/RunConcurrentRepresentationTests.kt b/src/jvm/test/org/jetbrains/kotlinx/lincheck_test/representation/RunConcurrentRepresentationTests.kt index 032142de3d..25b5cc8abc 100644 --- a/src/jvm/test/org/jetbrains/kotlinx/lincheck_test/representation/RunConcurrentRepresentationTests.kt +++ b/src/jvm/test/org/jetbrains/kotlinx/lincheck_test/representation/RunConcurrentRepresentationTests.kt @@ -180,6 +180,33 @@ class VariableReadWriteRunConcurrentRepresentationTest : BaseRunConcurrentRepres } } +class AnonymousObjectRunConcurrentRepresentationTest : BaseRunConcurrentRepresentationTest( + if (isJdk8) "run_concurrent_test/anonymous_object_jdk8.txt" else "run_concurrent_test/anonymous_object.txt" +) { + // use static fields to avoid local object optimizations + companion object { + @JvmField var runnable: Runnable? = null + @JvmField var x = 0 + } + + // use the interface to additionally check that KT-16727 bug is handled: + // https://youtrack.jetbrains.com/issue/KT-16727/ + interface I { + fun test() = object : Runnable { + override fun run() { + x++ + } + } + } + + @Suppress("UNUSED_VARIABLE") + override fun block() { + runnable = (object : I {}).test() + runnable!!.run() + check(false) + } +} + // TODO investigate difference for trace debugger (Evgeniy Moiseenko) class CustomThreadsRunConcurrentRepresentationTest : BaseRunConcurrentRepresentationTest( if (isInTraceDebuggerMode) { diff --git a/src/jvm/test/resources/expected_logs/run_concurrent_test/anonymous_object.txt b/src/jvm/test/resources/expected_logs/run_concurrent_test/anonymous_object.txt new file mode 100644 index 0000000000..bf1d24fc72 --- /dev/null +++ b/src/jvm/test/resources/expected_logs/run_concurrent_test/anonymous_object.txt @@ -0,0 +1,28 @@ += Concurrent test failed = + +java.lang.IllegalStateException: Check failed. + at org.jetbrains.kotlinx.lincheck_test.representation.AnonymousObjectRunConcurrentRepresentationTest.block(RunConcurrentRepresentationTests.kt:206) + at org.jetbrains.kotlinx.lincheck_test.representation.AnonymousObjectRunConcurrentRepresentationTest.block(RunConcurrentRepresentationTests.kt:183) + at org.jetbrains.kotlinx.lincheck_test.representation.BaseRunConcurrentRepresentationTest$testRunWithModelChecker$result$1$1.invoke(RunConcurrentRepresentationTests.kt:40) + at java.base/java.lang.Thread.run(Thread.java:840) + +The following interleaving leads to the error: +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Thread 1 | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| AnonymousObjectRunConcurrentRepresentationTest#1.block(): threw IllegalStateException at BaseRunConcurrentRepresentationTest$testRunWithModelChecker$result$1$1.invoke(RunConcurrentRepresentationTests.kt:40) | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | + +Detailed trace: +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Thread 1 | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| AnonymousObjectRunConcurrentRepresentationTest#1.block(): threw IllegalStateException at BaseRunConcurrentRepresentationTest$testRunWithModelChecker$result$1$1.invoke(RunConcurrentRepresentationTests.kt:40) | +| block(): threw IllegalStateException at AnonymousObjectRunConcurrentRepresentationTest.block(RunConcurrentRepresentationTests.kt:183) | +| AnonymousObjectRunConcurrentRepresentationTest.runnable = test$1 at AnonymousObjectRunConcurrentRepresentationTest.block(RunConcurrentRepresentationTests.kt:204) | +| AnonymousObjectRunConcurrentRepresentationTest.runnable ➜ test$1 at AnonymousObjectRunConcurrentRepresentationTest.block(RunConcurrentRepresentationTests.kt:205) | +| test$1.run() at AnonymousObjectRunConcurrentRepresentationTest.block(RunConcurrentRepresentationTests.kt:205) | +| AnonymousObjectRunConcurrentRepresentationTest.x ➜ 0 at AnonymousObjectRunConcurrentRepresentationTest$I$test$1.run(RunConcurrentRepresentationTests.kt:197) | +| AnonymousObjectRunConcurrentRepresentationTest.x = 1 at AnonymousObjectRunConcurrentRepresentationTest$I$test$1.run(RunConcurrentRepresentationTests.kt:197) | +| result: IllegalStateException #1 | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | diff --git a/src/jvm/test/resources/expected_logs/run_concurrent_test/anonymous_object_jdk8.txt b/src/jvm/test/resources/expected_logs/run_concurrent_test/anonymous_object_jdk8.txt new file mode 100644 index 0000000000..9d09f420ae --- /dev/null +++ b/src/jvm/test/resources/expected_logs/run_concurrent_test/anonymous_object_jdk8.txt @@ -0,0 +1,28 @@ += Concurrent test failed = + +java.lang.IllegalStateException: Check failed. + at org.jetbrains.kotlinx.lincheck_test.representation.AnonymousObjectRunConcurrentRepresentationTest.block(RunConcurrentRepresentationTests.kt:206) + at org.jetbrains.kotlinx.lincheck_test.representation.AnonymousObjectRunConcurrentRepresentationTest.block(RunConcurrentRepresentationTests.kt:183) + at org.jetbrains.kotlinx.lincheck_test.representation.BaseRunConcurrentRepresentationTest$testRunWithModelChecker$result$1$1.invoke(RunConcurrentRepresentationTests.kt:40) + at java.lang.Thread.run(Thread.java:750) + +The following interleaving leads to the error: +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Thread 1 | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| AnonymousObjectRunConcurrentRepresentationTest#1.block(): threw IllegalStateException at BaseRunConcurrentRepresentationTest$testRunWithModelChecker$result$1$1.invoke(RunConcurrentRepresentationTests.kt:40) | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | + +Detailed trace: +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Thread 1 | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| AnonymousObjectRunConcurrentRepresentationTest#1.block(): threw IllegalStateException at BaseRunConcurrentRepresentationTest$testRunWithModelChecker$result$1$1.invoke(RunConcurrentRepresentationTests.kt:40) | +| block(): threw IllegalStateException at AnonymousObjectRunConcurrentRepresentationTest.block(RunConcurrentRepresentationTests.kt:183) | +| AnonymousObjectRunConcurrentRepresentationTest.runnable = at AnonymousObjectRunConcurrentRepresentationTest.block(RunConcurrentRepresentationTests.kt:204) | +| AnonymousObjectRunConcurrentRepresentationTest.runnable ➜ at AnonymousObjectRunConcurrentRepresentationTest.block(RunConcurrentRepresentationTests.kt:205) | +| .run() at AnonymousObjectRunConcurrentRepresentationTest.block(RunConcurrentRepresentationTests.kt:205) | +| AnonymousObjectRunConcurrentRepresentationTest.x ➜ 0 at AnonymousObjectRunConcurrentRepresentationTest$I$test$1.run(RunConcurrentRepresentationTests.kt:197) | +| AnonymousObjectRunConcurrentRepresentationTest.x = 1 at AnonymousObjectRunConcurrentRepresentationTest$I$test$1.run(RunConcurrentRepresentationTests.kt:197) | +| result: IllegalStateException #1 | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |