Fixed KT-15963 (coroutines machinery should not use equals on results)

This commit is contained in:
Roman Elizarov
2017-01-26 10:29:49 +03:00
committed by Denis Zharkov
parent c362a9154b
commit 8dc318dd92
4 changed files with 97 additions and 29 deletions
+11 -11
View File
@@ -111,7 +111,7 @@ internal abstract class CoroutineImpl(private val resultContinuation: Continuati
protected fun doResumeWrapper() {
try {
result = doResume()
if (result != COROUTINE_SUSPENDED) {
if (result !== COROUTINE_SUSPENDED) {
val data = result
result = COROUTINE_SUSPENDED
resultContinuation.resume(data)
@@ -136,11 +136,11 @@ internal class SafeContinuation<in T> internal constructor(private val delegate:
private var result: Any? = UNDECIDED
override fun resume(value: T) {
when (result) {
UNDECIDED -> {
when {
result === UNDECIDED -> {
result = value
}
COROUTINE_SUSPENDED -> {
result === COROUTINE_SUSPENDED -> {
result = RESUMED
delegate.resume(value)
}
@@ -151,11 +151,11 @@ internal class SafeContinuation<in T> internal constructor(private val delegate:
}
override fun resumeWithException(exception: Throwable) {
when (result) {
UNDECIDED -> {
when {
result === UNDECIDED -> {
result = Fail(exception)
}
COROUTINE_SUSPENDED -> {
result === COROUTINE_SUSPENDED -> {
result = RESUMED
delegate.resumeWithException(exception)
}
@@ -166,15 +166,15 @@ internal class SafeContinuation<in T> internal constructor(private val delegate:
}
internal fun getResult(): Any? {
if (result == UNDECIDED) {
if (result === UNDECIDED) {
result = COROUTINE_SUSPENDED
}
val result = this.result
return when (result) {
RESUMED -> {
return when {
result === RESUMED -> {
COROUTINE_SUSPENDED // already called continuation, indicate SUSPENDED upstream
}
is Fail -> {
result is Fail -> {
throw result.exception
}
else -> {
@@ -157,19 +157,16 @@ internal class SafeContinuation<in T> @PublishedApi internal constructor(private
companion object {
@Suppress("UNCHECKED_CAST")
@JvmStatic
private val RESULT_UPDATER = AtomicReferenceFieldUpdater.newUpdater<SafeContinuation<*>, Any?>(
private val RESULT = AtomicReferenceFieldUpdater.newUpdater<SafeContinuation<*>, Any?>(
SafeContinuation::class.java, Any::class.java as Class<Any?>, "result")
}
private fun cas(expect: Any?, update: Any?): Boolean =
RESULT_UPDATER.compareAndSet(this, expect, update)
override fun resume(value: T) {
while (true) { // lock-free loop
val result = this.result // atomic read
when (result) {
UNDECIDED -> if (cas(UNDECIDED, value)) return
COROUTINE_SUSPENDED -> if (cas(COROUTINE_SUSPENDED, RESUMED)) {
when {
result === UNDECIDED -> if (RESULT.compareAndSet(this, UNDECIDED, value)) return
result === COROUTINE_SUSPENDED -> if (RESULT.compareAndSet(this, COROUTINE_SUSPENDED, RESUMED)) {
delegate.resume(value)
return
}
@@ -181,9 +178,9 @@ internal class SafeContinuation<in T> @PublishedApi internal constructor(private
override fun resumeWithException(exception: Throwable) {
while (true) { // lock-free loop
val result = this.result // atomic read
when (result) {
UNDECIDED -> if (cas(UNDECIDED, Fail(exception))) return
COROUTINE_SUSPENDED -> if (cas(COROUTINE_SUSPENDED, RESUMED)) {
when {
result === UNDECIDED -> if (RESULT.compareAndSet(this, UNDECIDED, Fail(exception))) return
result === COROUTINE_SUSPENDED -> if (RESULT.compareAndSet(this, COROUTINE_SUSPENDED, RESUMED)) {
delegate.resumeWithException(exception)
return
}
@@ -195,14 +192,14 @@ internal class SafeContinuation<in T> @PublishedApi internal constructor(private
@PublishedApi
internal fun getResult(): Any? {
var result = this.result // atomic read
if (result == UNDECIDED) {
if (cas(UNDECIDED, COROUTINE_SUSPENDED)) return COROUTINE_SUSPENDED
if (result === UNDECIDED) {
if (RESULT.compareAndSet(this, UNDECIDED, COROUTINE_SUSPENDED)) return COROUTINE_SUSPENDED
result = this.result // reread volatile var
}
when (result) {
RESUMED -> return COROUTINE_SUSPENDED // already called continuation, indicate COROUTINE_SUSPENDED upstream
is Fail -> throw result.exception
else -> return result // either COROUTINE_SUSPENDED or data
when {
result === RESUMED -> return COROUTINE_SUSPENDED // already called continuation, indicate SUSPENDED_MARKER upstream
result is Fail -> throw result.exception
else -> return result // either SUSPENDED_MARKER or data
}
}
}
@@ -51,7 +51,7 @@ abstract class CoroutineImpl(
override fun resume(value: Any?) {
try {
val result = doResume(value, null)
if (result != COROUTINE_SUSPENDED)
if (result !== COROUTINE_SUSPENDED)
completion!!.resume(result)
} catch (e: Throwable) {
completion!!.resumeWithException(e)
@@ -61,7 +61,7 @@ abstract class CoroutineImpl(
override fun resumeWithException(exception: Throwable) {
try {
val result = doResume(null, exception)
if (result != COROUTINE_SUSPENDED)
if (result !== COROUTINE_SUSPENDED)
completion!!.resume(result)
} catch (e: Throwable) {
completion!!.resumeWithException(e)
@@ -0,0 +1,71 @@
/*
* Copyright 2010-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package test.coroutines
import kotlin.test.*
import org.junit.Test
import kotlin.coroutines.*
/**
* Test to ensure that coroutine machinery does not call equals/hashCode/toString anywhere.
*/
class CoroutinesReferenceValuesTest {
class BadClass {
override fun equals(other: Any?): Boolean = error("equals")
override fun hashCode(): Int = error("hashCode")
override fun toString(): String = error("toString")
}
var counter = 0
// tail-suspend function via suspendCoroutine (test SafeContinuation)
suspend fun getBadClassViaSuspend(): BadClass = suspendCoroutine { cont ->
counter++
cont.resume(BadClass())
}
// state machine
suspend fun checkBadClassTwice() {
assertTrue(getBadClassViaSuspend() is BadClass)
assertTrue(getBadClassViaSuspend() is BadClass)
}
fun <T> suspend(block: suspend () -> T) = block
@Test
fun testBadClass() {
val bad = suspend {
checkBadClassTwice()
getBadClassViaSuspend()
}
var result: BadClass? = null
bad.startCoroutine(object : Continuation<BadClass> {
override val context: CoroutineContext = EmptyCoroutineContext
override fun resume(value: BadClass) {
assertTrue(result == null)
result = value
}
override fun resumeWithException(exception: Throwable) {
throw exception
}
})
assertTrue(result is BadClass)
assertEquals(3, counter)
}
}