From 29b0b30031af8033cba93ca9fec48b1aa6b95f95 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Mon, 30 Jan 2017 14:09:41 +0300 Subject: [PATCH] Make createCoroutine return a safe continuation #KT-15718 Fixed --- .../box/coroutines/createCoroutineSafe.kt | 25 ++++++++++ .../codegen/box/coroutines/illegalState.kt | 9 +++- .../coroutines/createCoroutineSafe.txt | 30 ++++++++++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 6 +++ .../codegen/BlackBoxCodegenTestGenerated.java | 6 +++ js/js.libraries/src/core/coroutines.kt | 26 ++++++---- .../semantics/JsCodegenBoxTestGenerated.java | 6 +++ .../experimental/CoroutinesLibrary.kt | 48 +++++++++++-------- 8 files changed, 126 insertions(+), 30 deletions(-) create mode 100644 compiler/testData/codegen/box/coroutines/createCoroutineSafe.kt create mode 100644 compiler/testData/codegen/light-analysis/coroutines/createCoroutineSafe.txt diff --git a/compiler/testData/codegen/box/coroutines/createCoroutineSafe.kt b/compiler/testData/codegen/box/coroutines/createCoroutineSafe.kt new file mode 100644 index 00000000000..063b58847f9 --- /dev/null +++ b/compiler/testData/codegen/box/coroutines/createCoroutineSafe.kt @@ -0,0 +1,25 @@ +// WITH_RUNTIME +// WITH_COROUTINES +import kotlin.coroutines.experimental.* +import kotlin.coroutines.experimental.intrinsics.* + +fun builder(c: suspend () -> Unit) { + val x = c.createCoroutine(EmptyContinuation) + + x.resume(Unit) + x.resume(Unit) +} + +fun box(): String { + var result = "" + + try { + builder { + result = "OK" + } + } catch (e: IllegalStateException) { + return result + } + + return "fail: $result" +} diff --git a/compiler/testData/codegen/box/coroutines/illegalState.kt b/compiler/testData/codegen/box/coroutines/illegalState.kt index 6e550df64df..90c7c2bec8e 100644 --- a/compiler/testData/codegen/box/coroutines/illegalState.kt +++ b/compiler/testData/codegen/box/coroutines/illegalState.kt @@ -15,9 +15,14 @@ fun builder1(c: suspend () -> Unit) { fun builder2(c: suspend () -> Unit) { val continuation = c.createCoroutine(EmptyContinuation) - val declaredField = continuation.javaClass.superclass.getDeclaredField("label") + + val delegateField = continuation.javaClass.getDeclaredField("delegate") + delegateField.setAccessible(true) + val originalContinuation = delegateField.get(continuation) + + val declaredField = originalContinuation.javaClass.superclass.getDeclaredField("label") declaredField.setAccessible(true) - declaredField.set(continuation, -3) + declaredField.set(originalContinuation, -3) continuation.resume(Unit) } diff --git a/compiler/testData/codegen/light-analysis/coroutines/createCoroutineSafe.txt b/compiler/testData/codegen/light-analysis/coroutines/createCoroutineSafe.txt new file mode 100644 index 00000000000..682bfca1606 --- /dev/null +++ b/compiler/testData/codegen/light-analysis/coroutines/createCoroutineSafe.txt @@ -0,0 +1,30 @@ +@kotlin.Metadata +public final class CoroutineUtilKt { + public final static @org.jetbrains.annotations.NotNull method handleExceptionContinuation(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function1): kotlin.coroutines.experimental.Continuation + public final static @org.jetbrains.annotations.NotNull method handleResultContinuation(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function1): kotlin.coroutines.experimental.Continuation +} + +@kotlin.Metadata +public final class CreateCoroutineSafeKt { + public final static @org.jetbrains.annotations.NotNull method box(): java.lang.String + public final static method builder(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function1): void +} + +@kotlin.Metadata +public class EmptyContinuation { + public final static field Companion: EmptyContinuation.Companion + private final @org.jetbrains.annotations.NotNull field context: kotlin.coroutines.experimental.CoroutineContext + inner class EmptyContinuation/Companion + public @synthetic.kotlin.jvm.GeneratedByJvmOverloads method (): void + public method (@org.jetbrains.annotations.NotNull p0: kotlin.coroutines.experimental.CoroutineContext): void + public synthetic method (p0: kotlin.coroutines.experimental.CoroutineContext, p1: int, p2: kotlin.jvm.internal.DefaultConstructorMarker): void + public @org.jetbrains.annotations.NotNull method getContext(): kotlin.coroutines.experimental.CoroutineContext + public method resume(@org.jetbrains.annotations.Nullable p0: java.lang.Object): void + public method resumeWithException(@org.jetbrains.annotations.NotNull p0: java.lang.Throwable): void +} + +@kotlin.Metadata +public final static class EmptyContinuation/Companion { + inner class EmptyContinuation/Companion + private method (): void +} diff --git a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 1a17cbcc8bf..73b3b7d0dd1 100644 --- a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -4667,6 +4667,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes doTest(fileName); } + @TestMetadata("createCoroutineSafe.kt") + public void testCreateCoroutineSafe() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/createCoroutineSafe.kt"); + doTest(fileName); + } + @TestMetadata("createCoroutinesOnManualInstances.kt") public void testCreateCoroutinesOnManualInstances() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/createCoroutinesOnManualInstances.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index bf1907e8ba6..3d307a81032 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -4667,6 +4667,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } + @TestMetadata("createCoroutineSafe.kt") + public void testCreateCoroutineSafe() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/createCoroutineSafe.kt"); + doTest(fileName); + } + @TestMetadata("createCoroutinesOnManualInstances.kt") public void testCreateCoroutinesOnManualInstances() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/createCoroutinesOnManualInstances.kt"); diff --git a/js/js.libraries/src/core/coroutines.kt b/js/js.libraries/src/core/coroutines.kt index 90a7f27e62f..637f22d7d92 100644 --- a/js/js.libraries/src/core/coroutines.kt +++ b/js/js.libraries/src/core/coroutines.kt @@ -28,7 +28,11 @@ import kotlin.coroutines.experimental.intrinsics.* public fun (suspend R.() -> T).createCoroutine( receiver: R, completion: Continuation -): Continuation = this.asDynamic()(receiver, completion, true) +): Continuation = + SafeContinuation( + this.asDynamic()(receiver, completion, true), + COROUTINE_SUSPENDED + ) /** * Starts coroutine with receiver type [R] and result type [T]. @@ -40,8 +44,7 @@ public fun (suspend R.() -> T).startCoroutine( receiver: R, completion: Continuation ) { - val coroutine: Continuation = this.asDynamic()(receiver, completion, true) - coroutine.resume(null) + createCoroutine(receiver, completion).resume(Unit) } /** @@ -53,7 +56,11 @@ public fun (suspend R.() -> T).startCoroutine( @SinceKotlin("1.1") public fun (suspend () -> T).createCoroutine( completion: Continuation -): Continuation = this.asDynamic()(completion, true) +): Continuation = + SafeContinuation( + this.asDynamic()(completion, true), + COROUTINE_SUSPENDED + ) /** * Starts coroutine without receiver and with result type [T]. @@ -64,8 +71,7 @@ public fun (suspend () -> T).createCoroutine( public fun (suspend () -> T).startCoroutine( completion: Continuation ) { - val coroutine: Continuation = this.asDynamic()(completion, true) - coroutine.resume(null) + createCoroutine(completion).resume(Unit) } /** @@ -129,11 +135,15 @@ private val UNDECIDED: Any? = Any() private val RESUMED: Any? = Any() private class Fail(val exception: Throwable) -internal class SafeContinuation internal constructor(private val delegate: Continuation) : Continuation { +internal class SafeContinuation +@PublishedApi internal constructor( + private val delegate: Continuation, + initialResult: Any? = UNDECIDED +) : Continuation { public override val context: CoroutineContext get() = delegate.context - private var result: Any? = UNDECIDED + private var result: Any? = initialResult override fun resume(value: T) { when { diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index b3c5104352d..ad3e73e6fbf 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -5352,6 +5352,12 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { doTest(fileName); } + @TestMetadata("createCoroutineSafe.kt") + public void testCreateCoroutineSafe() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/createCoroutineSafe.kt"); + doTest(fileName); + } + @TestMetadata("createCoroutinesOnManualInstances.kt") public void testCreateCoroutinesOnManualInstances() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/createCoroutinesOnManualInstances.kt"); diff --git a/libraries/stdlib/src/kotlin/coroutines/experimental/CoroutinesLibrary.kt b/libraries/stdlib/src/kotlin/coroutines/experimental/CoroutinesLibrary.kt index 7539d5ff4b8..1ecd680f07d 100644 --- a/libraries/stdlib/src/kotlin/coroutines/experimental/CoroutinesLibrary.kt +++ b/libraries/stdlib/src/kotlin/coroutines/experimental/CoroutinesLibrary.kt @@ -36,14 +36,16 @@ import kotlin.coroutines.experimental.jvm.internal.interceptContinuationIfNeeded public fun (suspend R.() -> T).createCoroutine( receiver: R, completion: Continuation -): Continuation { - if (this !is CoroutineImpl) { - return buildContinuationByInvokeCall(completion) { - (this as Function2, Any?>).invoke(receiver, completion) - } - } - return ((this as CoroutineImpl).create(receiver, completion) as CoroutineImpl).facade -} +): Continuation = + SafeContinuation( + if (this !is CoroutineImpl) + buildContinuationByInvokeCall(completion) { + (this as Function2, Any?>).invoke(receiver, completion) + } + else + ((this as CoroutineImpl).create(receiver, completion) as CoroutineImpl).facade, + COROUTINE_SUSPENDED + ) /** * Starts coroutine with receiver type [R] and result type [T]. @@ -69,14 +71,16 @@ public fun (suspend R.() -> T).startCoroutine( @Suppress("UNCHECKED_CAST") public fun (suspend () -> T).createCoroutine( completion: Continuation -): Continuation { - if (this !is CoroutineImpl) { - return buildContinuationByInvokeCall(completion) { - (this as Function1, Any?>).invoke(completion) - } - } - return ((this as CoroutineImpl).create(completion) as CoroutineImpl).facade -} +): Continuation = + SafeContinuation( + if (this !is CoroutineImpl) + buildContinuationByInvokeCall(completion) { + (this as Function1, Any?>).invoke(completion) + } + else + ((this as CoroutineImpl).create(completion) as CoroutineImpl).facade, + COROUTINE_SUSPENDED + ) /** * Starts coroutine without receiver and with result type [T]. @@ -147,12 +151,16 @@ private val RESUMED: Any? = Any() private class Fail(val exception: Throwable) @PublishedApi -internal class SafeContinuation @PublishedApi internal constructor(private val delegate: Continuation) : Continuation { +internal class SafeContinuation +@PublishedApi internal constructor( + private val delegate: Continuation, + initialResult: Any? = UNDECIDED +) : Continuation { public override val context: CoroutineContext get() = delegate.context @Volatile - private var result: Any? = UNDECIDED + private var result: Any? = initialResult companion object { @Suppress("UNCHECKED_CAST") @@ -197,9 +205,9 @@ internal class SafeContinuation @PublishedApi internal constructor(private result = this.result // reread volatile var } when { - result === RESUMED -> return COROUTINE_SUSPENDED // already called continuation, indicate SUSPENDED_MARKER upstream + result === RESUMED -> return COROUTINE_SUSPENDED // already called continuation, indicate COROUTINE_SUSPENDED upstream result is Fail -> throw result.exception - else -> return result // either SUSPENDED_MARKER or data + else -> return result // either COROUTINE_SUSPENDED or data } } }