Skip to content

Commit

Permalink
Fix instrumentation of newly loaded classes (#525)
Browse files Browse the repository at this point in the history
  • Loading branch information
@eupp
eupp authored Feb 18, 2025
1 parent 552c2f6 commit b03e042
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 12 deletions.
19 changes: 16 additions & 3 deletions src/jvm/main/org/jetbrains/kotlinx/lincheck/strategy/managed/ObjectLabelFactory.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<unknown>` class name.
.getOrElse {
"<unknown>"
}
return objectName
}

private fun objectName(obj: Any): String {
Expand Down
27 changes: 18 additions & 9 deletions src/jvm/main/org/jetbrains/kotlinx/lincheck/transformation/LincheckJavaAgent.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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()
}
Expand Down Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions ...st/org/jetbrains/kotlinx/lincheck_test/representation/RunConcurrentRepresentationTests.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,33 @@ class VariableReadWriteRunConcurrentRepresentationTest : BaseRunConcurrentRepres
}
}

class AnonymousObjectRunConcurrentRepresentationTest : BaseRunConcurrentRepresentationTest<Unit>(
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<Unit>(
if (isInTraceDebuggerMode) {
Expand Down
28 changes: 28 additions & 0 deletions src/jvm/test/resources/expected_logs/run_concurrent_test/anonymous_object.txt
View file
Original file line number Diff line number Diff line change
@@ -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 |
| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
28 changes: 28 additions & 0 deletions src/jvm/test/resources/expected_logs/run_concurrent_test/anonymous_object_jdk8.txt
View file
Original file line number Diff line number Diff line change
@@ -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 = <unknown> at AnonymousObjectRunConcurrentRepresentationTest.block(RunConcurrentRepresentationTests.kt:204) |
| AnonymousObjectRunConcurrentRepresentationTest.runnable ➜ <unknown> at AnonymousObjectRunConcurrentRepresentationTest.block(RunConcurrentRepresentationTests.kt:205) |
| <unknown>.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 |
| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |

0 comments on commit b03e042

Please sign in to comment.