Skip to content

Commit

Permalink
Review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
alefedor committed Aug 24, 2020
1 parent 8b4eab8 commit da8d2e7
Show file tree
Hide file tree
Showing 9 changed files with 91 additions and 162 deletions.
43 changes: 17 additions & 26 deletions src/main/java/org/jetbrains/kotlinx/lincheck/Result.kt
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package org.jetbrains.kotlinx.lincheck

import java.io.ByteArrayOutputStream
import java.io.ObjectOutputStream
import java.io.Serializable
import java.io.*
import kotlin.coroutines.*

/*
Expand Down Expand Up @@ -47,34 +45,36 @@ sealed class Result {
/**
* Type of result used if the actor invocation returns any value.
*/
data class ValueResult @JvmOverloads constructor(val value: Any?, override val wasSuspended: Boolean = false) : Result() {
override fun toString() = wasSuspendedPrefix + "$value"
}

/**
* Type of result used if the actor invocation returns a transformed object that implements Serializable.
*/
class SerializedResult(private val value: Any, override val wasSuspended: Boolean) : Result() {
class ValueResult @JvmOverloads constructor(val value: Any?, override val wasSuspended: Boolean = false) : Result() {
private lateinit var serializedObject: ByteArray
private val valueTransformed: Boolean
get() = value?.javaClass?.classLoader is TransformationClassLoader

override fun toString() = wasSuspendedPrefix + "$value"

override fun equals(other: Any?): Boolean {
if (this === other) return true
// check class equality by names, because classes can be loaded via different loaders
if (javaClass.name != other?.javaClass?.name) return false
other as SerializedResult
other as ValueResult
if (wasSuspended != other.wasSuspended) return false
if (!getSerializedObject().contentEquals(other.getSerializedObject())) return false
val valueTransformed = valueTransformed
if (valueTransformed != other.valueTransformed) return false
if (valueTransformed) {
if (!getSerializedObject().contentEquals(other.getSerializedObject())) return false
} else {
if (value != other.value) return false
}
return true
}

override fun hashCode(): Int {
return (if (wasSuspended) 1 else 0) + 2 * getSerializedObject().hashCode()
}
override fun hashCode(): Int = (if (wasSuspended) 1 else 0) + 2 * value.hashCode()

private fun getSerializedObject(): ByteArray {
if (!::serializedObject.isInitialized) {
if (value !is Serializable) {
throw IllegalArgumentException("Actor results should either be basic " +
"(java.lang.String, int, Integer, etc, and corresponding classes in other JVM languages) or implement Serializable")
}
val byteArrayStream = ByteArrayOutputStream()
ObjectOutputStream(byteArrayStream).use {
it.writeObject(value)
Expand Down Expand Up @@ -122,15 +122,6 @@ data class ExceptionResult private constructor(val tClazz: Class<out Throwable>,
@JvmSynthetic
fun createExceptionResult(tClazz: Class<out Throwable>) = ExceptionResult.create(tClazz, false)

@JvmOverloads
@JvmSynthetic
fun createResultFromObject(res: Any?, wasSuspended: Boolean = false) = when {
res == null || TransformationClassLoader.doNotTransform(res.javaClass.name) -> ValueResult(res, wasSuspended)
res is Serializable -> SerializedResult(res, wasSuspended)
else -> throw IllegalArgumentException("Actor results should either be basic " +
"(java.lang.String, int, Integer, etc, and corresponding classes in other JVM languages) or implement Serializable")
}

/**
* Type of result used if the actor invocation suspended the thread and did not get the final result yet
* though it can be resumed later
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
*/

import org.jetbrains.kotlinx.lincheck.runner.Runner;
import org.jetbrains.kotlinx.lincheck.strategy.ManagedStrategyHolder;
import org.jetbrains.kotlinx.lincheck.strategy.Strategy;
import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
Expand Down Expand Up @@ -69,8 +70,7 @@ static boolean doNotTransform(String className) {
return className == null ||
(className.startsWith("org.jetbrains.kotlinx.lincheck.") &&
!className.startsWith("org.jetbrains.kotlinx.lincheck.test.") &&
!className.endsWith("ManagedStrategyHolder") &&
!className.endsWith("TransformableUtils")) ||
!className.equals(ManagedStrategyHolder.class.getName())) ||
className.startsWith("sun.") ||
className.startsWith("java.") ||
className.startsWith("jdk.internal.") ||
Expand Down
38 changes: 15 additions & 23 deletions src/main/java/org/jetbrains/kotlinx/lincheck/Utils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,13 @@ package org.jetbrains.kotlinx.lincheck

import kotlinx.coroutines.CancellableContinuation
import org.jetbrains.kotlinx.lincheck.CancellableContinuationHolder.storedLastCancellableCont
import org.jetbrains.kotlinx.lincheck.execution.ExecutionResult
import org.jetbrains.kotlinx.lincheck.execution.ExecutionScenario
import org.jetbrains.kotlinx.lincheck.execution.*
import org.jetbrains.kotlinx.lincheck.runner.FixedActiveThreadsExecutor
import org.jetbrains.kotlinx.lincheck.verifier.DummySequentialSpecification
import java.io.*
import java.lang.IllegalArgumentException
import java.lang.ref.WeakReference
import java.lang.reflect.InvocationTargetException
import java.lang.reflect.Method
import java.lang.*
import java.lang.ref.*
import java.lang.reflect.*
import java.util.*
import kotlin.coroutines.Continuation
import kotlin.coroutines.intrinsics.COROUTINE_SUSPENDED
Expand Down Expand Up @@ -82,7 +80,7 @@ internal fun executeActor(
try {
val m = getMethod(instance, actor.method)
val args = (if (actor.isSuspendable) actor.arguments + completion else actor.arguments)
.map { it.transportToLoader(instance.javaClass.classLoader) }
.map { it.convertForLoader(instance.javaClass.classLoader) }
val res = m.invoke(instance, *args.toTypedArray())
return if (m.returnType.isAssignableFrom(Void.TYPE)) VoidResult else createLincheckResult(res)
} catch (invE: Throwable) {
Expand All @@ -103,7 +101,7 @@ internal fun executeActor(
}

internal inline fun executeValidationFunctions(instance: Any, validationFunctions: List<Method>,
onError: (functionName: String, exception: Throwable) -> Unit) {
onError: (functionName: String, exception: Throwable) -> Unit) {
for (f in validationFunctions) {
val validationException = executeValidationFunction(instance, f)
if (validationException != null) {
Expand Down Expand Up @@ -139,16 +137,10 @@ private fun getMethod(instance: Any, method: Method): Method {
/**
* Finds a method corresponding to [name] and [parameterTypes] ignoring difference in loaders for [parameterTypes].
*/
private fun Class<out Any>.getMethod(name: String, parameterTypes: Array<Class<out Any>>): Method {
this.methods.forEach {
if (it.name == name && it.parameterTypes.size == parameterTypes.size) {
val sameParameters = parameterTypes.indices.all { i -> it.parameterTypes[i].name == parameterTypes[i].name }
if (sameParameters)
return it
}
}
throw NoSuchMethodException("${getName()}.$name(${parameterTypes.joinToString(",")})");
}
private fun Class<out Any>.getMethod(name: String, parameterTypes: Array<Class<out Any>>): Method =
methods.find { method ->
method.name == name && method.parameterTypes.map { it.name } == parameterTypes.map { it.name }
} ?: throw NoSuchMethodException("${getName()}.$name(${parameterTypes.joinToString(",")})")

/**
* Creates [Result] of corresponding type from any given value.
Expand All @@ -168,7 +160,7 @@ internal fun createLincheckResult(res: Any?, wasSuspended: Boolean = false) = wh
res != null && res is Throwable -> ExceptionResult.create(res.javaClass, wasSuspended)
res === COROUTINE_SUSPENDED -> Suspended
res is kotlin.Result<Any?> -> res.toLinCheckResult(wasSuspended)
else -> createResultFromObject(res, wasSuspended)
else -> ValueResult(res, wasSuspended)
}

private fun kotlin.Result<Any?>.toLinCheckResult(wasSuspended: Boolean) =
Expand All @@ -177,7 +169,7 @@ private fun kotlin.Result<Any?>.toLinCheckResult(wasSuspended: Boolean) =
is Unit -> if (wasSuspended) SuspendedVoidResult else VoidResult
// Throwable was returned as a successful result
is Throwable -> ValueResult(value::class.java, wasSuspended)
else -> createResultFromObject(value, wasSuspended)
else -> ValueResult(value, wasSuspended)
}
} else ExceptionResult.create(exceptionOrNull()!!.let { it::class.java }, wasSuspended)

Expand Down Expand Up @@ -229,13 +221,13 @@ fun storeCancellableContinuation(cont: CancellableContinuation<*>) {
}
}

internal fun transportScenarioToLoader(scenario: ExecutionScenario, loader: ClassLoader) =
internal fun convertForLoader(scenario: ExecutionScenario, loader: ClassLoader) =
ExecutionScenario(
scenario.initExecution,
scenario.parallelExecution.map { it.map { actor ->
Actor(
actor.method,
actor.arguments.map { it.transportToLoader(loader) },
actor.arguments.map { it.convertForLoader(loader) },
actor.handledExceptions,
actor.cancelOnSuspension,
actor.allowExtraSuspension
Expand All @@ -244,7 +236,7 @@ internal fun transportScenarioToLoader(scenario: ExecutionScenario, loader: Clas
scenario.postExecution
)

private fun Any?.transportToLoader(loader: ClassLoader): Any? =
private fun Any?.convertForLoader(loader: ClassLoader): Any? =
when {
this == null || TransformationClassLoader.doNotTransform(this.javaClass.name) -> this
this is Serializable -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import java.util.*;
import java.util.concurrent.atomic.*;

import static org.jetbrains.kotlinx.lincheck.UtilsKt.convertForLoader;

/**
* Runner determines how to run your concurrent test. In order to support techniques
* like fibers, it may require code transformation, so {@link #needsTransformation()}
Expand All @@ -49,7 +51,7 @@ public abstract class Runner {
protected Runner(Strategy strategy, Class<?> testClass, List<Method> validationFunctions) {
this.classLoader = (this.needsTransformation() || strategy.needsTransformation()) ?
new TransformationClassLoader(strategy, this) : new ExecutionClassLoader();
this.scenario = UtilsKt.transportScenarioToLoader(strategy.getScenario(), classLoader);
this.scenario = convertForLoader(strategy.getScenario(), classLoader);
this.testClass = loadClass(testClass.getTypeName());
this.validationFunctions = validationFunctions;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,12 @@ public class TestThreadExecutionGenerator {

private static final String INSTANCE = "INSTANCE";

private static final Type VALUE_RESULT_TYPE = getType(ValueResult.class);
private static final Method VALUE_RESULT_TYPE_CONSTRUCTOR = new Method("<init>", VOID_TYPE, new Type[] {OBJECT_TYPE});

private static final Type EXCEPTION_RESULT_TYPE = getType(ExceptionResult.class);
private static final Type RESULT_KT_TYPE = getType(ResultKt.class);
private static final Method RESULT_KT_CREATE_EXCEPTION_RESULT_METHOD = new Method("createExceptionResult", EXCEPTION_RESULT_TYPE, new Type[] {CLASS_TYPE});
private static final Method RESULT_KT_CREATE_RESULT_FROM_OBJECT_METHOD = new Method("createResultFromObject", RESULT_TYPE, new Type[] {OBJECT_TYPE});

private static final Type RESULT_ARRAY_TYPE = getType(Result[].class);

Expand Down Expand Up @@ -210,6 +212,13 @@ private static void generateRun(ClassVisitor cv, Type testType, int iThread, Lis
mv.loadThis();
mv.getField(TEST_THREAD_EXECUTION_TYPE, "runner", RUNNER_TYPE);
mv.checkCast(PARALLEL_THREADS_RUNNER_TYPE);
} else {
// Prepare to create non-processed value result of actor invocation (in case of no suspendable actors in scenario)
// Load type of result
if (actor.getMethod().getReturnType() != void.class) {
mv.newInstance(VALUE_RESULT_TYPE);
mv.visitInsn(DUP);
}
}
// Load test instance
mv.loadThis();
Expand All @@ -234,8 +243,7 @@ private static void generateRun(ClassVisitor cv, Type testType, int iThread, Lis
if (actor.getMethod().getReturnType() == void.class) {
createVoidResult(actor, mv);
} else {
// Create result of actor invocation (in case of no suspendable actors in scenario)
mv.invokeStatic(RESULT_KT_TYPE, RESULT_KT_CREATE_RESULT_FROM_OBJECT_METHOD);
mv.invokeConstructor(VALUE_RESULT_TYPE, VALUE_RESULT_TYPE_CONSTRUCTOR);
}
}
// Store result to array
Expand Down Expand Up @@ -376,4 +384,4 @@ private static void pushArgumentOnStack(GeneratorAdapter mv, List<Object> objArg
objArgs.add(arg);
}
}
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,28 @@
package org.jetbrains.kotlinx.lincheck.test.transformation

import org.jetbrains.kotlinx.lincheck.Options
import org.jetbrains.kotlinx.lincheck.annotations.Operation
import org.jetbrains.kotlinx.lincheck.annotations.Param
import org.jetbrains.kotlinx.lincheck.annotations.*
import org.jetbrains.kotlinx.lincheck.paramgen.ParameterGenerator
import org.jetbrains.kotlinx.lincheck.test.AbstractLincheckTest
import java.io.Serializable
import java.util.concurrent.atomic.*

class SerializableResultTest : AbstractLincheckTest() {
private val counter = AtomicReference(ValueHolder(0))

@Operation
fun getAndSet(key: Int) = counter.getAndSet(ValueHolder(key))

override fun extractState(): Any = counter.get().value

override fun <O : Options<O, *>> O.customize() {
iterations(1)
actorsBefore(0)
actorsAfter(0)
}
}


@Param(name = "key", gen = ValueHolderGen::class)
class SerializableParameterTest : AbstractLincheckTest() {
private val counter = AtomicInteger(0)
Expand All @@ -43,12 +58,13 @@ class SerializableParameterTest : AbstractLincheckTest() {
}

override fun extractState(): Any = counter.get()

class ValueHolder(val value: Int) : Serializable
}

class ValueHolderGen(conf: String) : ParameterGenerator<SerializableParameterTest.ValueHolder> {
override fun generate(): SerializableParameterTest.ValueHolder {
return listOf(SerializableParameterTest.ValueHolder(1), SerializableParameterTest.ValueHolder(2)).random()

class ValueHolder(val value: Int) : Serializable

class ValueHolderGen(conf: String) : ParameterGenerator<ValueHolder> {
override fun generate(): ValueHolder {
return listOf(ValueHolder(1), ValueHolder(2)).random()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,34 @@ package org.jetbrains.kotlinx.lincheck.test.transformation

import org.jetbrains.kotlinx.lincheck.Options
import org.jetbrains.kotlinx.lincheck.annotations.Operation
import org.jetbrains.kotlinx.lincheck.strategy.*
import org.jetbrains.kotlinx.lincheck.test.AbstractLincheckTest

class ExpectedTransformedExceptionTest : AbstractLincheckTest() {
var canEnterForbiddenBlock = false

@Operation(handleExceptionsAsResult = [CustomException::class])
fun operation() {
canEnterForbiddenBlock = true
canEnterForbiddenBlock = false
if (canEnterForbiddenBlock)
throw CustomException()
}
fun operation(): Unit = throw CustomException()

override fun <O : Options<O, *>> O.customize() {
iterations(1)
}

override fun extractState(): Any = canEnterForbiddenBlock
override fun extractState(): Any = 0 // constant state
}

class UnexpectedTransformedExceptionTest : AbstractLincheckTest(UnexpectedExceptionFailure::class) {
@Volatile
var throwException = false

@Operation
fun operation(): Int {
throwException = true
throwException = false
if (throwException)
throw CustomException()
return 0
}

override fun extractState(): Any = 0 // constant state
}

internal class CustomException : Throwable()
Loading

0 comments on commit da8d2e7

Please sign in to comment.