Transform lambdas correctly and enhance the GPMC API shape (#545)

* Lambdas are transformed correctly
* Use `Runnable` in `runConcurrentTest` for better interop with Java
* Java API is supported

---------

Signed-off-by: Evgeniy Moiseenko <[email protected]>
Co-authored-by: Evgeniy Moiseenko <[email protected]>

Author: ndkoval

bootstrap/src/sun/nio/ch/lincheck/Injections.java

@@ -132,6 +132,8 @@ public static boolean inAnalyzedCode() {
     public static boolean beforeThreadFork(Thread forkedThread) {
         // TestThread is handled separately
         if (forkedThread instanceof TestThread) return false;
+        // If thread is started return immediately, as in this case, JVM will throw an `IllegalThreadStateException`
+        if (forkedThread.getState() != Thread.State.NEW) return false;
         ThreadDescriptor descriptor = ThreadDescriptor.getCurrentThreadDescriptor();
         if (descriptor == null) {
             return false;

src/jvm/main/org/jetbrains/kotlinx/lincheck/IdeaPlugin.kt

@@ -398,7 +398,7 @@ private fun visualize(strategy: ManagedStrategy) = runCatching {
     val lincheckThreads = runner.executor.threads
     val testObject = runner.testInstance.takeIf {
         // in general-purpose model checking mode `testObject` is null
-        it !is GeneralPurposeModelCheckingWrapper<*>
+        it !is GeneralPurposeModelCheckingWrapper
     }
     visualizeInstance(testObject,
         objectToNumberMap = createObjectToNumberMapAsArray(testObject),

src/jvm/main/org/jetbrains/kotlinx/lincheck/Lincheck.kt

@@ -8,6 +8,8 @@
  * with this file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
+@file:JvmName("Lincheck")
+
 package org.jetbrains.kotlinx.lincheck
 
 import org.jetbrains.kotlinx.lincheck.execution.*
@@ -31,13 +33,15 @@ annotation class ExperimentalModelCheckingAPI
  * @param block lambda which body will be a target for the interleavings exploration.
  */
 @ExperimentalModelCheckingAPI
-fun <R> runConcurrentTest(
+@JvmOverloads
+fun runConcurrentTest(
     invocations: Int = DEFAULT_INVOCATIONS_COUNT,
-    block: () -> R
+    block: Runnable
 ) {
+    // TODO: do not use DSL to avoid spending 300ms in Kotlin Reflection
     val scenario = scenario {
         parallel {
-            thread { actor(GeneralPurposeModelCheckingWrapper<R>::run, block) }
+            thread { actor(GeneralPurposeModelCheckingWrapper::runGPMCTest, block) }
         }
     }
 
@@ -74,8 +78,8 @@ fun <R> runConcurrentTest(
     }
 }
 
-internal class GeneralPurposeModelCheckingWrapper<R>() {
-    fun run(block: () -> R) = block()
+internal class GeneralPurposeModelCheckingWrapper {
+    fun runGPMCTest(block: Runnable) = block.run()
 }
 
 /**

src/jvm/main/org/jetbrains/kotlinx/lincheck/Reporter.kt

@@ -413,7 +413,7 @@ internal fun StringBuilder.appendFailure(failure: LincheckFailure): StringBuilde
 
 internal fun isGeneralPurposeModelCheckingScenario(scenario: ExecutionScenario): Boolean {
     val actor = scenario.parallelExecution.getOrNull(0)?.getOrNull(0)
-    return (actor?.method == GeneralPurposeModelCheckingWrapper<*>::run.javaMethod)
+    return (actor?.method == GeneralPurposeModelCheckingWrapper::runGPMCTest.javaMethod)
 }
 
 private data class ExecutionResultsRepresentationData(
@@ -632,7 +632,7 @@ internal fun collectExceptionStackTraces(executionResult: ExecutionResult): Exce
             }
             val stackTrace = exception.stackTrace
                 // filter lincheck methods
-                .filter { LINCHECK_PACKAGE_NAME !in it.className }
+                .filter { !isInLincheckPackage(it.className) }
             exceptionStackTraces[exception] = ExceptionNumberAndStacktrace(index + 1, stackTrace)
         }
     return ExceptionStackTracesResult(exceptionStackTraces)
@@ -643,11 +643,10 @@ private fun Throwable.isInternalLincheckBug(): Boolean {
     // so we filter out stack trace elements of these runner routines
     val testStackTrace = stackTrace.takeWhile { LINCHECK_RUNNER_PACKAGE_NAME !in it.className }
     // collect Lincheck functions from the stack trace
-    val lincheckStackFrames = testStackTrace.filter { LINCHECK_PACKAGE_NAME in it.className }
+    val lincheckStackFrames = testStackTrace.filter { isInLincheckPackage(it.className) }
     // special handling of `cancelByLincheck` primitive and general purpose model checking function call
-    val lincheckLegalStackFrames = listOf("cancelByLincheck", "GeneralPurposeModelCheckingWrapper.run")
-    if (lincheckStackFrames.size == 1 &&
-        lincheckLegalStackFrames.any { it in lincheckStackFrames[0].toString() }) {
+    val lincheckLegalStackFrames = listOf("cancelByLincheck", "runGPMCTest")
+    if (lincheckStackFrames.all { it.methodName in lincheckLegalStackFrames }) {
         return false
     }
     // otherwise, if the stack trace contains any Lincheck functions, we classify it as a Lincheck bug

src/jvm/main/org/jetbrains/kotlinx/lincheck/Utils.kt

@@ -243,8 +243,12 @@ internal fun Class<*>.findField(fieldName: String): Field {
  */
 internal class LincheckInternalBugException(cause: Throwable): Exception(cause)
 
+internal fun isInLincheckPackage(className: String) =
+    className.startsWith(LINCHECK_PACKAGE_NAME) || className.startsWith(LINCHECK_BOOTSTRAP_PACKAGE_NAME)
+
 internal const val LINCHECK_PACKAGE_NAME = "org.jetbrains.kotlinx.lincheck."
 internal const val LINCHECK_RUNNER_PACKAGE_NAME = "org.jetbrains.kotlinx.lincheck.runner."
+internal const val LINCHECK_BOOTSTRAP_PACKAGE_NAME = "sun.nio.ch.lincheck."
 
 internal fun <T> Class<T>.newDefaultInstance(): T {
     @Suppress("UNCHECKED_CAST")

src/jvm/main/org/jetbrains/kotlinx/lincheck/strategy/managed/TraceReporter.kt

@@ -391,7 +391,7 @@ private fun removeSyntheticFieldAccessTracePoints(trace: List<TracePoint>) {
         }
 }
 
-private fun isSyntheticFieldAccess(methodName: String): Boolean = 
+private fun isSyntheticFieldAccess(methodName: String): Boolean =
     methodName.contains("access\$get") || methodName.contains("access\$set")
 
 /**
@@ -435,7 +435,7 @@ private fun compressCallStackTrace(
             removed.add(nextElement.id)
             continue
         }
-        
+
         // Check if current and next are compressible
         if (isCompressiblePair(currentElement.tracePoint.methodName, nextElement.tracePoint.methodName)) {
             // Combine fields of next and current, and store in current
@@ -474,7 +474,7 @@ private fun actorNodeResultRepresentation(result: Result?, failure: LincheckFail
 }
 
 /**
- * Used by [compressCallStackTrace] to remove the two `invoke()` lines at the beginning of 
+ * Used by [compressCallStackTrace] to remove the two `invoke()` lines at the beginning of
  * a user-defined thread trace.
  */
 private fun isUserThreadStart(currentElement: CallStackTraceElement, nextElement: CallStackTraceElement): Boolean =
@@ -529,7 +529,7 @@ private fun isDefaultPair(currentName: String, nextName: String): Boolean =
  *
  */
 private fun isAccessPair(currentName: String, nextName: String): Boolean =
-    currentName == "access$${nextName}" 
+    currentName == "access$${nextName}"
 
 /**
  * Helper class to provider execution results, including a validation function result
@@ -710,7 +710,7 @@ internal abstract class TraceInnerNode(prefixProvider: PrefixProvider, iThread:
     fun addInternalEvent(node: TraceNode) {
         _internalEvents.add(node)
     }
-    
+
 }
 
 internal class CallNode(
@@ -755,12 +755,12 @@ internal class ActorNode(
         val actorRepresentation =
             prefix + actorRepresentation + if (resultRepresentation != null) ": $resultRepresentation" else ""
         traceRepresentation.add(TraceEventRepresentation(iThread, actorRepresentation))
-        
+
         if (!shouldBeExpanded(verboseTrace)) {
             if (isCustomThreadActor) directChildren.forEach { it.addRepresentationTo(traceRepresentation, true) }
             lastState?.let { traceRepresentation.add(stateEventRepresentation(iThread, it)) }
             return lastInternalEvent.next
-        } 
+        }
         return next
     }
 }

src/jvm/main/org/jetbrains/kotlinx/lincheck/transformation/LincheckJavaAgent.kt

@@ -239,13 +239,13 @@ internal object LincheckJavaAgent {
      *
      * The function is called upon a test instance creation, to ensure that all the classes related to it are transformed.
      *
-     * @param testInstance the object to be transformed
+     * @param obj the object to be transformed
      */
-    fun ensureObjectIsTransformed(testInstance: Any) {
+    fun ensureObjectIsTransformed(obj: Any) {
         if (INSTRUMENT_ALL_CLASSES) {
             return
         }
-        ensureObjectIsTransformed(testInstance, Collections.newSetFromMap(IdentityHashMap()))
+        ensureObjectIsTransformed(obj, Collections.newSetFromMap(IdentityHashMap()))
     }
 
     /**
@@ -269,27 +269,37 @@ internal object LincheckJavaAgent {
      * @param processedObjects A set of processed objects to avoid infinite recursion.
      */
     private fun ensureObjectIsTransformed(obj: Any, processedObjects: MutableSet<Any>) {
-        if (obj is Array<*>) {
-            obj.filterNotNull().forEach { ensureObjectIsTransformed(it, processedObjects) }
-            return
-        }
+        var clazz: Class<*> = obj.javaClass
+        val className = clazz.name
 
-        if (!instrumentation.isModifiableClass(obj.javaClass) || !shouldTransform(obj.javaClass.name, instrumentationMode)) {
-            return
+        when {
+            isJavaLambdaClass(className) -> {
+                ensureClassHierarchyIsTransformed(getJavaLambdaEnclosingClass(className))
+            }
+            obj is Array<*> -> {
+                obj.filterNotNull().forEach {
+                    ensureObjectIsTransformed(it, processedObjects)
+                }
+                return
+            }
+            else -> {
+                if (!instrumentation.isModifiableClass(obj.javaClass) ||
+                    !shouldTransform(obj.javaClass.name, instrumentationMode)
+                ) {
+                    return
+                }
+                ensureClassHierarchyIsTransformed(clazz)
+            }
         }
 
         if (processedObjects.contains(obj)) return
         processedObjects += obj
 
-        var clazz: Class<*> = obj.javaClass
-
-        ensureClassHierarchyIsTransformed(clazz)
-
         while (true) {
             clazz.declaredFields
                 .filter { !it.type.isPrimitive }
                 .filter { !Modifier.isStatic(it.modifiers) }
-                .mapNotNull { readFieldViaUnsafe(obj, it, Unsafe::getObject) }
+                .mapNotNull { readFieldSafely(obj, it).getOrNull() }
                 .forEach {
                     ensureObjectIsTransformed(it, processedObjects)
                 }
@@ -304,17 +314,16 @@ internal object LincheckJavaAgent {
      * @param processedObjects Set of objects that have already been processed to prevent duplicate transformation.
      */
     private fun ensureClassHierarchyIsTransformed(clazz: Class<*>, processedObjects: MutableSet<Any>) {
-        if (instrumentation.isModifiableClass(clazz) && shouldTransform(clazz.name, instrumentationMode)) {
+        if (!shouldTransform(clazz.name, instrumentationMode)) return
+        if (instrumentation.isModifiableClass(clazz)) {
             instrumentedClasses += clazz.name
             instrumentation.retransformClasses(clazz)
-        } else {
-            return
         }
         // Traverse static fields.
         clazz.declaredFields
             .filter { !it.type.isPrimitive }
             .filter { Modifier.isStatic(it.modifiers) }
-            .mapNotNull { readFieldViaUnsafe(null, it, Unsafe::getObject) }
+            .mapNotNull { readFieldSafely(null, it).getOrNull() }
             .forEach {
                 ensureObjectIsTransformed(it, processedObjects)
             }

src/jvm/main/org/jetbrains/kotlinx/lincheck/transformation/TransformationUtils.kt

@@ -502,6 +502,20 @@ internal fun isCoroutineStateMachineClass(className: String): Boolean {
 
 private val isCoroutineStateMachineClassMap = ConcurrentHashMap<String, Boolean>()
 
+/**
+ * Test if the given class name corresponds to a Java lambda class.
+ */
+internal fun isJavaLambdaClass(className: String): Boolean =
+    className.contains("\$\$Lambda")
+
+/**
+ * Extracts and returns the enclosing class name of a Java lambda class.
+ */
+internal fun getJavaLambdaEnclosingClass(className: String): String {
+    require(isJavaLambdaClass(className)) { "Not a Java lambda class: $className" }
+    return className.substringBefore("\$\$Lambda")
+}
+
 /**
  * Tests if the provided [className] contains `"ClassLoader"` as a substring.
  */

src/jvm/main/org/jetbrains/kotlinx/lincheck/util/Logger.kt

@@ -32,24 +32,35 @@ internal object Logger {
 
     inline fun error(lazyMessage: () -> String) = log(LoggingLevel.ERROR, lazyMessage)
 
-    fun error(e: Throwable) = error {
-        StringWriter().use {
-            e.printStackTrace(PrintWriter(it))
-        }.toString()
-    }
-
     inline fun warn(lazyMessage: () -> String) = log(LoggingLevel.WARN, lazyMessage)
 
     inline fun info(lazyMessage: () -> String) = log(LoggingLevel.INFO, lazyMessage)
 
     inline fun debug(lazyMessage: () -> String) = log(LoggingLevel.DEBUG, lazyMessage)
 
+    fun error(e: Throwable) = log(LoggingLevel.ERROR, e)
+
+    fun warn(e: Throwable) = log(LoggingLevel.WARN, e)
+
+    fun info(e: Throwable) = log(LoggingLevel.INFO, e)
+
+    fun debug(e: Throwable) = log(LoggingLevel.DEBUG, e)
+
     private inline fun log(logLevel: LoggingLevel, lazyMessage: () -> String) {
         if (logLevel >= this.logLevel) {
             write(logLevel, lazyMessage(), logWriter)
         }
     }
 
+    private fun log(logLevel: LoggingLevel, throwable: Throwable) {
+        log(logLevel) {
+            StringWriter().use { writer ->
+                throwable.printStackTrace(PrintWriter(writer))
+                writer.toString()
+            }
+        }
+    }
+
     private fun write(logLevel: LoggingLevel, s: String, writer: Writer) {
         try {
             writer.write("[${getCurrentTimeStamp()}] [${logLevel.name}] $s$LINE_SEPARATOR")

src/jvm/main/org/jetbrains/kotlinx/lincheck/util/UnsafeHolder.kt

@@ -67,14 +67,25 @@ internal fun readFieldViaUnsafe(obj: Any?, field: Field): Any? {
  * Reads a [field] of the owner object [obj] via Unsafe,
  * in case of failure fallbacks into reading the field via reflection.
  */
-internal fun readFieldSafely(obj: Any?, field: Field): kotlin.Result<Any?> {
+internal fun readFieldSafely(obj: Any?, field: Field): kotlin.Result<Any?> =
     // we wrap an unsafe read into `runCatching` to handle `UnsupportedOperationException`,
     // which can be thrown, for instance, when attempting to read
     // a field of a hidden or record class (starting from Java 15);
     // in this case we fall back to read via reflection
-    return runCatching { readFieldViaUnsafe(obj, field) }
-        .recoverCatching { field.apply { isAccessible = true }.get(obj) }
-}
+    runCatching {
+        readFieldViaUnsafe(obj, field)
+    }
+    .onFailure { exception ->
+        Logger.debug { "Failed to read field ${field.name} via Unsafe" }
+        Logger.debug(exception)
+    }
+    .recoverCatching {
+        field.apply { isAccessible = true }.get(obj)
+    }
+    .onFailure { exception ->
+        Logger.debug { "Failed to read field ${field.name} via reflection." }
+        Logger.debug(exception)
+    }
 
 internal fun readArrayElementViaUnsafe(arr: Any, index: Int): Any? {
     val offset = getArrayElementOffsetViaUnsafe(arr, index)