Reflection: fix exceptions on concurrent access to some properties
Using `ReflectionProperties.lazy` is incorrect because it allows several threads to observe different resulting values if they're computing it simultaneously (unlike `lazy(PUBLICATION)`, which always returns the value that "won the race"). In the case of property delegates, for example, if we're invoking `isAccessible = true` and then `getDelegate()` concurrently, it might happen that when some thread invokes `getDelegate()`, it gets the underlying Field object which was written by another thread and which has not yet been made accessible, leading to IllegalPropertyDelegateAccessException. #KT-27585 Fixed
This commit is contained in:
committed by
Space Team
parent
e8f95a3376
commit
99b38ccb74
@@ -0,0 +1,38 @@
|
||||
import java.util.concurrent.atomic.AtomicInteger
|
||||
import java.util.concurrent.atomic.AtomicReference
|
||||
import java.util.concurrent.CyclicBarrier
|
||||
import kotlin.concurrent.thread
|
||||
import kotlin.reflect.full.*
|
||||
import kotlin.reflect.jvm.*
|
||||
|
||||
const val N_THREADS = 50
|
||||
|
||||
class C {
|
||||
private fun function() {}
|
||||
}
|
||||
|
||||
fun main() {
|
||||
val instance = C()
|
||||
val reference = C::class.functions.single { it.name == "function" }
|
||||
|
||||
val gate = CyclicBarrier(N_THREADS + 1)
|
||||
var fail = AtomicReference<Throwable?>(null)
|
||||
var finished = AtomicInteger(0)
|
||||
for (i in 0 until N_THREADS) {
|
||||
thread {
|
||||
gate.await()
|
||||
reference.isAccessible = true
|
||||
try {
|
||||
reference.javaMethod!!.invoke(instance)
|
||||
} catch (e: Throwable) {
|
||||
fail.set(e)
|
||||
}
|
||||
finished.incrementAndGet()
|
||||
}
|
||||
}
|
||||
|
||||
gate.await()
|
||||
|
||||
while (finished.get() != N_THREADS) Thread.sleep(25L)
|
||||
fail.get()?.let { throw it }
|
||||
}
|
||||
@@ -0,0 +1,39 @@
|
||||
import java.util.concurrent.atomic.AtomicInteger
|
||||
import java.util.concurrent.atomic.AtomicReference
|
||||
import java.util.concurrent.CyclicBarrier
|
||||
import kotlin.concurrent.thread
|
||||
import kotlin.reflect.jvm.*
|
||||
|
||||
const val N_THREADS = 50
|
||||
|
||||
class Delegate {
|
||||
operator fun getValue(x: Any?, y: Any?): String = "OK"
|
||||
operator fun setValue(x: Any?, y: Any?, z: String) {}
|
||||
}
|
||||
|
||||
var property by Delegate()
|
||||
|
||||
fun main() {
|
||||
val reference = ::property
|
||||
|
||||
val gate = CyclicBarrier(N_THREADS + 1)
|
||||
var fail = AtomicReference<Throwable?>(null)
|
||||
var finished = AtomicInteger(0)
|
||||
for (i in 0 until N_THREADS) {
|
||||
thread {
|
||||
gate.await()
|
||||
reference.isAccessible = true
|
||||
try {
|
||||
reference.getDelegate()!!
|
||||
} catch (e: Throwable) {
|
||||
fail.set(e)
|
||||
}
|
||||
finished.incrementAndGet()
|
||||
}
|
||||
}
|
||||
|
||||
gate.await()
|
||||
|
||||
while (finished.get() != N_THREADS) Thread.sleep(25L)
|
||||
fail.get()?.let { throw it }
|
||||
}
|
||||
@@ -34,25 +34,15 @@ class ReflectionIntegrationTest : KtUsefulTestCase() {
|
||||
|
||||
val lib = CompilerTestUtil.compileJvmLibrary(File("$root/test.kt"))
|
||||
|
||||
val javaHome = System.getProperty("java.home")
|
||||
val javaExe = File(javaHome, "bin/java.exe").takeIf(File::exists)
|
||||
?: File(javaHome, "bin/java").takeIf(File::exists)
|
||||
?: error("Can't find 'java' executable in $javaHome")
|
||||
|
||||
val command = arrayOf(
|
||||
javaExe.absolutePath,
|
||||
runJava(
|
||||
"-ea",
|
||||
"-classpath",
|
||||
tmpdir.absolutePath,
|
||||
"Main",
|
||||
lib.absolutePath,
|
||||
ForTestCompileRuntime.runtimeJarForTests().absolutePath,
|
||||
ForTestCompileRuntime.reflectJarForTests().absolutePath
|
||||
ForTestCompileRuntime.reflectJarForTests().absolutePath,
|
||||
)
|
||||
|
||||
val process = ProcessBuilder(*command).inheritIO().start()
|
||||
process.waitFor(1, TimeUnit.MINUTES)
|
||||
assertEquals(0, process.exitValue())
|
||||
}
|
||||
|
||||
// This test checks that simultaneous access to kotlin-reflect from different threads works, in case the URLClassLoader instance is
|
||||
@@ -95,4 +85,41 @@ class ReflectionIntegrationTest : KtUsefulTestCase() {
|
||||
|
||||
error.get()?.let { throw it }
|
||||
}
|
||||
|
||||
fun testConcurrentAccessToPropertyDelegate() {
|
||||
compileAndRunProgram(KtTestUtil.getTestDataPathBase() + "/reflection/concurrentAccessToPropertyDelegate")
|
||||
}
|
||||
|
||||
fun testConcurrentAccessToPrivateFunction() {
|
||||
compileAndRunProgram(KtTestUtil.getTestDataPathBase() + "/reflection/concurrentAccessToPrivateFunction")
|
||||
}
|
||||
|
||||
private fun compileAndRunProgram(root: String) {
|
||||
val lib = CompilerTestUtil.compileJvmLibrary(File("$root/test.kt"))
|
||||
|
||||
runJava(
|
||||
"-ea",
|
||||
"-classpath",
|
||||
listOf(
|
||||
ForTestCompileRuntime.runtimeJarForTests().absolutePath,
|
||||
ForTestCompileRuntime.reflectJarForTests().absolutePath,
|
||||
lib.absolutePath,
|
||||
).joinToString(File.pathSeparator),
|
||||
"TestKt",
|
||||
)
|
||||
}
|
||||
|
||||
private fun runJava(vararg args: String) {
|
||||
val javaHome = System.getProperty("java.home")
|
||||
val javaExe = File(javaHome, "bin/java.exe").takeIf(File::exists)
|
||||
?: File(javaHome, "bin/java").takeIf(File::exists)
|
||||
?: error("Can't find 'java' executable in $javaHome")
|
||||
|
||||
val process = ProcessBuilder(javaExe.absolutePath, *args).start()
|
||||
process.waitFor(1, TimeUnit.MINUTES)
|
||||
val stderr = process.errorStream.reader().readText()
|
||||
val stdout = process.inputStream.reader().readText()
|
||||
val exitCode = process.exitValue()
|
||||
assertEquals("Program exited with exit code $exitCode.\nStdout:\n$stdout\nStderr:\n$stderr", 0, exitCode)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user