Fix improper handling of coroutine interceptors
Don't use coroutine facade in following cases: * When calling coroutine functions in non-suspend mode (i.e. suspend flag is either false or skipped). * When passing continuation from suspend function to suspend function. Use facade only for corresponding intrinsics. Reason: interceptor should not be called on each suspend function invocation, it should be called after resume from innermost suspend function. Fix KT-17067. The example provided with the issue is very similar to dispatchResume.kt which passes after these changes. Add test to prove that KT-17446 is no more reproducible.
This commit is contained in:
+1
-1
@@ -1,6 +1,6 @@
|
|||||||
// WITH_RUNTIME
|
// WITH_RUNTIME
|
||||||
// WITH_COROUTINES
|
// WITH_COROUTINES
|
||||||
// IGNORE_BACKEND: JS, NATIVE
|
// IGNORE_BACKEND: NATIVE
|
||||||
import kotlin.coroutines.experimental.*
|
import kotlin.coroutines.experimental.*
|
||||||
import kotlin.coroutines.experimental.intrinsics.*
|
import kotlin.coroutines.experimental.intrinsics.*
|
||||||
import kotlin.test.assertEquals
|
import kotlin.test.assertEquals
|
||||||
|
|||||||
@@ -0,0 +1,48 @@
|
|||||||
|
// IGNORE_BACKEND: NATIVE
|
||||||
|
// WITH_RUNTIME
|
||||||
|
// WITH_COROUTINES
|
||||||
|
import kotlin.coroutines.experimental.*
|
||||||
|
import kotlin.coroutines.experimental.intrinsics.suspendCoroutineOrReturn
|
||||||
|
import kotlin.coroutines.experimental.intrinsics.COROUTINE_SUSPENDED
|
||||||
|
|
||||||
|
fun box(): String {
|
||||||
|
async {
|
||||||
|
log += "1;"
|
||||||
|
wait()
|
||||||
|
log += "2;"
|
||||||
|
}
|
||||||
|
|
||||||
|
while (postponed != null) {
|
||||||
|
log += "suspended;"
|
||||||
|
postponed!!()
|
||||||
|
}
|
||||||
|
|
||||||
|
if (log != "1;wait2;suspended;wait;suspended;2;") return "fail: $log"
|
||||||
|
|
||||||
|
return "OK"
|
||||||
|
}
|
||||||
|
|
||||||
|
fun async(f: suspend () -> Unit) {
|
||||||
|
f.startCoroutine(handleResultContinuation {
|
||||||
|
postponed = null
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
suspend fun wait(): Unit {
|
||||||
|
wait2()
|
||||||
|
log += "wait;"
|
||||||
|
return suspendCoroutineOrReturn { c ->
|
||||||
|
postponed = { c.resume(Unit) }
|
||||||
|
COROUTINE_SUSPENDED
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
suspend fun wait2(): Unit = suspendCoroutineOrReturn { c ->
|
||||||
|
log += "wait2;"
|
||||||
|
postponed = { c.resume(Unit) }
|
||||||
|
COROUTINE_SUSPENDED
|
||||||
|
}
|
||||||
|
|
||||||
|
var postponed: (() -> Unit)? = { }
|
||||||
|
|
||||||
|
var log = ""
|
||||||
+1
-1
@@ -1,4 +1,4 @@
|
|||||||
// IGNORE_BACKEND: JS, NATIVE
|
// IGNORE_BACKEND: NATIVE
|
||||||
// WITH_RUNTIME
|
// WITH_RUNTIME
|
||||||
// WITH_COROUTINES
|
// WITH_COROUTINES
|
||||||
import kotlin.coroutines.experimental.*
|
import kotlin.coroutines.experimental.*
|
||||||
|
|||||||
+6
@@ -5150,6 +5150,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
|
|||||||
doTest(fileName);
|
doTest(fileName);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@TestMetadata("suspendCoroutineFromStateMachine.kt")
|
||||||
|
public void testSuspendCoroutineFromStateMachine() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendCoroutineFromStateMachine.kt");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
|
||||||
@TestMetadata("suspendDefaultImpl.kt")
|
@TestMetadata("suspendDefaultImpl.kt")
|
||||||
public void testSuspendDefaultImpl() throws Exception {
|
public void testSuspendDefaultImpl() throws Exception {
|
||||||
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendDefaultImpl.kt");
|
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendDefaultImpl.kt");
|
||||||
|
|||||||
@@ -5150,6 +5150,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
|
|||||||
doTest(fileName);
|
doTest(fileName);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@TestMetadata("suspendCoroutineFromStateMachine.kt")
|
||||||
|
public void testSuspendCoroutineFromStateMachine() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendCoroutineFromStateMachine.kt");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
|
||||||
@TestMetadata("suspendDefaultImpl.kt")
|
@TestMetadata("suspendDefaultImpl.kt")
|
||||||
public void testSuspendDefaultImpl() throws Exception {
|
public void testSuspendDefaultImpl() throws Exception {
|
||||||
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendDefaultImpl.kt");
|
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendDefaultImpl.kt");
|
||||||
|
|||||||
@@ -5150,6 +5150,12 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes
|
|||||||
doTest(fileName);
|
doTest(fileName);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@TestMetadata("suspendCoroutineFromStateMachine.kt")
|
||||||
|
public void testSuspendCoroutineFromStateMachine() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendCoroutineFromStateMachine.kt");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
|
||||||
@TestMetadata("suspendDefaultImpl.kt")
|
@TestMetadata("suspendDefaultImpl.kt")
|
||||||
public void testSuspendDefaultImpl() throws Exception {
|
public void testSuspendDefaultImpl() throws Exception {
|
||||||
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendDefaultImpl.kt");
|
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendDefaultImpl.kt");
|
||||||
|
|||||||
@@ -107,10 +107,8 @@ class CoroutineMetadata(
|
|||||||
val finallyPathName: JsName,
|
val finallyPathName: JsName,
|
||||||
val resultName: JsName,
|
val resultName: JsName,
|
||||||
val exceptionName: JsName,
|
val exceptionName: JsName,
|
||||||
val facadeName: JsName,
|
|
||||||
val baseClassRef: JsExpression,
|
val baseClassRef: JsExpression,
|
||||||
val suspendObjectRef: JsExpression,
|
val suspendObjectRef: JsExpression,
|
||||||
val isLambda: Boolean,
|
|
||||||
val hasController: Boolean,
|
val hasController: Boolean,
|
||||||
val hasReceiver: Boolean
|
val hasReceiver: Boolean
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -161,7 +161,7 @@ class CoroutineFunctionTransformer(private val program: JsProgram, private val f
|
|||||||
functionWithBody.parameters += JsParameter(suspendedName)
|
functionWithBody.parameters += JsParameter(suspendedName)
|
||||||
|
|
||||||
val instanceName = functionWithBody.scope.declareFreshName("instance")
|
val instanceName = functionWithBody.scope.declareFreshName("instance")
|
||||||
functionWithBody.body.statements += JsAstUtils.newVar(instanceName, JsNameRef(context.metadata.facadeName, instantiation))
|
functionWithBody.body.statements += JsAstUtils.newVar(instanceName, instantiation)
|
||||||
|
|
||||||
val invokeResume = JsReturn(JsInvocation(JsNameRef(context.metadata.doResumeName, instanceName.makeRef()), JsLiteral.NULL))
|
val invokeResume = JsReturn(JsInvocation(JsNameRef(context.metadata.doResumeName, instanceName.makeRef()), JsLiteral.NULL))
|
||||||
|
|
||||||
|
|||||||
@@ -213,12 +213,10 @@ public class JsInliner extends JsVisitorWithContextImpl {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private void inlineSuspendWithCurrentContinuation(@NotNull JsInvocation call, @NotNull JsContext context) {
|
private void inlineSuspendWithCurrentContinuation(@NotNull JsInvocation call, @NotNull JsContext context) {
|
||||||
JsInliningContext inliningContext = getInliningContext();
|
|
||||||
JsFunction containingFunction = inliningContext.function;
|
|
||||||
JsExpression lambda = call.getArguments().get(0);
|
JsExpression lambda = call.getArguments().get(0);
|
||||||
JsParameter continuationParam = containingFunction.getParameters().get(containingFunction.getParameters().size() - 1);
|
JsExpression continuationArg = call.getArguments().get(call.getArguments().size() - 1);
|
||||||
|
|
||||||
JsInvocation invocation = new JsInvocation(lambda, continuationParam.getName().makeRef());
|
JsInvocation invocation = new JsInvocation(lambda, continuationArg);
|
||||||
MetadataProperties.setSuspend(invocation, true);
|
MetadataProperties.setSuspend(invocation, true);
|
||||||
context.replaceMe(accept(invocation));
|
context.replaceMe(accept(invocation));
|
||||||
}
|
}
|
||||||
@@ -268,11 +266,7 @@ public class JsInliner extends JsVisitorWithContextImpl {
|
|||||||
private class JsInliningContext implements InliningContext {
|
private class JsInliningContext implements InliningContext {
|
||||||
private final FunctionContext functionContext;
|
private final FunctionContext functionContext;
|
||||||
|
|
||||||
@NotNull
|
|
||||||
public final JsFunction function;
|
|
||||||
|
|
||||||
JsInliningContext(@NotNull JsFunction function) {
|
JsInliningContext(@NotNull JsFunction function) {
|
||||||
this.function = function;
|
|
||||||
functionContext = new FunctionContext(function, functionReader) {
|
functionContext = new FunctionContext(function, functionReader) {
|
||||||
@Nullable
|
@Nullable
|
||||||
@Override
|
@Override
|
||||||
|
|||||||
@@ -37,9 +37,9 @@ public inline fun <R, T> (suspend R.() -> T).startCoroutineUninterceptedOrReturn
|
|||||||
public fun <R, T> (suspend R.() -> T).createCoroutineUnchecked(
|
public fun <R, T> (suspend R.() -> T).createCoroutineUnchecked(
|
||||||
receiver: R,
|
receiver: R,
|
||||||
completion: Continuation<T>
|
completion: Continuation<T>
|
||||||
): Continuation<Unit> = this.asDynamic()(receiver, completion, true)
|
): Continuation<Unit> = this.asDynamic()(receiver, completion, true).facade
|
||||||
|
|
||||||
@SinceKotlin("1.1")
|
@SinceKotlin("1.1")
|
||||||
public fun <T> (suspend () -> T).createCoroutineUnchecked(
|
public fun <T> (suspend () -> T).createCoroutineUnchecked(
|
||||||
completion: Continuation<T>
|
completion: Continuation<T>
|
||||||
): Continuation<Unit> = this.asDynamic()(completion, true)
|
): Continuation<Unit> = this.asDynamic()(completion, true).facade
|
||||||
|
|||||||
+8
-14
@@ -5865,6 +5865,12 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
|
|||||||
doTest(fileName);
|
doTest(fileName);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@TestMetadata("suspendCoroutineFromStateMachine.kt")
|
||||||
|
public void testSuspendCoroutineFromStateMachine() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendCoroutineFromStateMachine.kt");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
|
||||||
@TestMetadata("suspendDefaultImpl.kt")
|
@TestMetadata("suspendDefaultImpl.kt")
|
||||||
public void testSuspendDefaultImpl() throws Exception {
|
public void testSuspendDefaultImpl() throws Exception {
|
||||||
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendDefaultImpl.kt");
|
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendDefaultImpl.kt");
|
||||||
@@ -6143,13 +6149,7 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
|
|||||||
@TestMetadata("startCoroutineUninterceptedOrReturnInterception.kt")
|
@TestMetadata("startCoroutineUninterceptedOrReturnInterception.kt")
|
||||||
public void testStartCoroutineUninterceptedOrReturnInterception() throws Exception {
|
public void testStartCoroutineUninterceptedOrReturnInterception() throws Exception {
|
||||||
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/intrinsicSemantics/startCoroutineUninterceptedOrReturnInterception.kt");
|
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/intrinsicSemantics/startCoroutineUninterceptedOrReturnInterception.kt");
|
||||||
try {
|
doTest(fileName);
|
||||||
doTest(fileName);
|
|
||||||
}
|
|
||||||
catch (Throwable ignore) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that.");
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -6224,13 +6224,7 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
|
|||||||
@TestMetadata("dispatchResume.kt")
|
@TestMetadata("dispatchResume.kt")
|
||||||
public void testDispatchResume() throws Exception {
|
public void testDispatchResume() throws Exception {
|
||||||
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/dispatchResume.kt");
|
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/suspendFunctionAsCoroutine/dispatchResume.kt");
|
||||||
try {
|
doTest(fileName);
|
||||||
doTest(fileName);
|
|
||||||
}
|
|
||||||
catch (Throwable ignore) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that.");
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@TestMetadata("handleException.kt")
|
@TestMetadata("handleException.kt")
|
||||||
|
|||||||
+1
-1
@@ -102,7 +102,7 @@ abstract class AbstractDeclarationVisitor : TranslatorVisitor<Unit>() {
|
|||||||
|
|
||||||
if (descriptor.isSuspend) {
|
if (descriptor.isSuspend) {
|
||||||
if (descriptor.requiresStateMachineTransformation(context)) {
|
if (descriptor.requiresStateMachineTransformation(context)) {
|
||||||
function.fillCoroutineMetadata(context, descriptor, hasController = false, isLambda = false)
|
function.fillCoroutineMetadata(context, descriptor, hasController = false)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+1
-1
@@ -92,7 +92,7 @@ class LiteralFunctionTranslator(context: TranslationContext) : AbstractTranslato
|
|||||||
fun JsFunction.fillCoroutineMetadata(context: TranslationContext, descriptor: FunctionDescriptor) {
|
fun JsFunction.fillCoroutineMetadata(context: TranslationContext, descriptor: FunctionDescriptor) {
|
||||||
if (!descriptor.isSuspend) return
|
if (!descriptor.isSuspend) return
|
||||||
|
|
||||||
fillCoroutineMetadata(context, descriptor, hasController = descriptor.extensionReceiverParameter != null, isLambda = true)
|
fillCoroutineMetadata(context, descriptor, hasController = descriptor.extensionReceiverParameter != null)
|
||||||
}
|
}
|
||||||
|
|
||||||
fun ValueParameterDescriptorImpl.WithDestructuringDeclaration.translate(context: TranslationContext): JsVars {
|
fun ValueParameterDescriptorImpl.WithDestructuringDeclaration.translate(context: TranslationContext): JsVars {
|
||||||
|
|||||||
+7
-2
@@ -16,6 +16,7 @@
|
|||||||
|
|
||||||
package org.jetbrains.kotlin.js.translate.reference
|
package org.jetbrains.kotlin.js.translate.reference
|
||||||
|
|
||||||
|
import org.jetbrains.kotlin.backend.common.isBuiltInSuspendCoroutineOrReturn
|
||||||
import org.jetbrains.kotlin.builtins.KotlinBuiltIns
|
import org.jetbrains.kotlin.builtins.KotlinBuiltIns
|
||||||
import org.jetbrains.kotlin.builtins.PrimitiveType
|
import org.jetbrains.kotlin.builtins.PrimitiveType
|
||||||
import org.jetbrains.kotlin.descriptors.FunctionDescriptor
|
import org.jetbrains.kotlin.descriptors.FunctionDescriptor
|
||||||
@@ -154,8 +155,12 @@ class CallArgumentTranslator private constructor(
|
|||||||
|
|
||||||
val callableDescriptor = resolvedCall.resultingDescriptor
|
val callableDescriptor = resolvedCall.resultingDescriptor
|
||||||
if (callableDescriptor is FunctionDescriptor && callableDescriptor.isSuspend) {
|
if (callableDescriptor is FunctionDescriptor && callableDescriptor.isSuspend) {
|
||||||
val facadeName = context().getNameForDescriptor(TranslationUtils.getCoroutineProperty(context(), "facade"))
|
var continuationArg: JsExpression = TranslationUtils.translateContinuationArgument(context())
|
||||||
result.add(JsAstUtils.pureFqn(facadeName, TranslationUtils.translateContinuationArgument(context())))
|
if (callableDescriptor.original.isBuiltInSuspendCoroutineOrReturn()) {
|
||||||
|
val facadeName = context().getNameForDescriptor(TranslationUtils.getCoroutineProperty(context(), "facade"))
|
||||||
|
continuationArg = JsAstUtils.pureFqn(facadeName, continuationArg)
|
||||||
|
}
|
||||||
|
result.add(continuationArg)
|
||||||
}
|
}
|
||||||
|
|
||||||
removeLastUndefinedArguments(result)
|
removeLastUndefinedArguments(result)
|
||||||
|
|||||||
@@ -144,7 +144,7 @@ fun FunctionDescriptor.requiresStateMachineTransformation(context: TranslationCo
|
|||||||
fun JsFunction.fillCoroutineMetadata(
|
fun JsFunction.fillCoroutineMetadata(
|
||||||
context: TranslationContext,
|
context: TranslationContext,
|
||||||
descriptor: FunctionDescriptor,
|
descriptor: FunctionDescriptor,
|
||||||
hasController: Boolean, isLambda: Boolean
|
hasController: Boolean
|
||||||
) {
|
) {
|
||||||
if (!descriptor.requiresStateMachineTransformation(context)) return
|
if (!descriptor.requiresStateMachineTransformation(context)) return
|
||||||
|
|
||||||
@@ -166,9 +166,7 @@ fun JsFunction.fillCoroutineMetadata(
|
|||||||
finallyPathName = getCoroutinePropertyName("finallyPath"),
|
finallyPathName = getCoroutinePropertyName("finallyPath"),
|
||||||
resultName = getCoroutinePropertyName("result"),
|
resultName = getCoroutinePropertyName("result"),
|
||||||
exceptionName = getCoroutinePropertyName("exception"),
|
exceptionName = getCoroutinePropertyName("exception"),
|
||||||
facadeName = getCoroutinePropertyName("facade"),
|
|
||||||
hasController = hasController,
|
hasController = hasController,
|
||||||
isLambda = isLambda,
|
|
||||||
hasReceiver = descriptor.dispatchReceiverParameter != null
|
hasReceiver = descriptor.dispatchReceiverParameter != null
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user