From 860f99482ac4f40d70f8c6e365da187332489cfa Mon Sep 17 00:00:00 2001 From: Svyatoslav Scherbina Date: Mon, 22 Aug 2022 17:59:17 +0200 Subject: [PATCH] Native: prohibit calling suspend functions from `autoreleasepool {}` If a suspend function is called from `autoreleasepool {}` block, this might cause the autoreleasepool frame to be removed earlier than expected. See https://youtrack.jetbrains.com/issue/KT-50786 for more details. ^KT-50786 Fixed --- .../backend/konan/lower/InteropLowering.kt | 39 +++++++++++++++++++ .../tests/compilerChecks/t63.kt | 30 ++++++++++++++ .../tests/compilerChecks/t64.kt | 32 +++++++++++++++ .../tests/interop/objc/tests/kt50786.kt | 34 ++++++++++++++++ 4 files changed, 135 insertions(+) create mode 100644 kotlin-native/backend.native/tests/compilerChecks/t63.kt create mode 100644 kotlin-native/backend.native/tests/compilerChecks/t64.kt create mode 100644 kotlin-native/backend.native/tests/interop/objc/tests/kt50786.kt diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/InteropLowering.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/InteropLowering.kt index 7946ceece67..12a2dcde12b 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/InteropLowering.kt +++ b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/InteropLowering.kt @@ -36,6 +36,9 @@ import org.jetbrains.kotlin.ir.symbols.impl.IrSimpleFunctionSymbolImpl import org.jetbrains.kotlin.ir.symbols.impl.IrValueParameterSymbolImpl import org.jetbrains.kotlin.ir.types.* import org.jetbrains.kotlin.ir.util.* +import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid +import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid +import org.jetbrains.kotlin.ir.visitors.acceptVoid import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.name.Name @@ -682,6 +685,42 @@ private class InteropLoweringPart1(val context: Context) : BaseInteropIrTransfor } } + override fun visitBlock(expression: IrBlock): IrExpression { + if (expression is IrReturnableBlock && expression.inlineFunctionSymbol?.isAutoreleasepool() == true) { + // Prohibit calling suspend functions from `autoreleasepool {}` block. + // See https://youtrack.jetbrains.com/issue/KT-50786 for more details. + // Note: we can't easily check this in frontend, because we need to prohibit indirect cases like + /// inline fun myAutoreleasepool(block: () -> T) = autoreleasepool(block) + /// myAutoreleasepool { suspendHere() } + + expression.acceptVoid(object : IrElementVisitorVoid { + override fun visitElement(element: IrElement) { + element.acceptChildrenVoid(this) + } + + override fun visitCall(expression: IrCall) { + super.visitCall(expression) + + if (expression.symbol.owner.isSuspend) { + context.reportCompilationError( + "Calling suspend functions from `autoreleasepool {}` is prohibited, " + + "see https://youtrack.jetbrains.com/issue/KT-50786", + currentFile, + expression + ) + } + } + }) + } + return super.visitBlock(expression) + } + + private fun IrFunctionSymbol.isAutoreleasepool(): Boolean { + return this.owner.name.asString() == "autoreleasepool" && this.owner.parent.let { parent -> + parent is IrPackageFragment && parent.fqName == InteropFqNames.packageName + } + } + private fun IrBuilderWithScope.callAllocAndInit( classPtr: IrExpression, initMethodInfo: ObjCMethodInfo, diff --git a/kotlin-native/backend.native/tests/compilerChecks/t63.kt b/kotlin-native/backend.native/tests/compilerChecks/t63.kt new file mode 100644 index 00000000000..84ba827c764 --- /dev/null +++ b/kotlin-native/backend.native/tests/compilerChecks/t63.kt @@ -0,0 +1,30 @@ +import kotlinx.cinterop.* +import kotlin.coroutines.* + +open class EmptyContinuation(override val context: CoroutineContext = EmptyCoroutineContext) : Continuation { + companion object : EmptyContinuation() + override fun resumeWith(result: Result) { result.getOrThrow() } +} + +lateinit var continuation: Continuation + +suspend fun suspendHere(): Unit = suspendCoroutine { cont -> + continuation = cont +} + + +fun startCoroutine(block: suspend () -> Unit) { + block.startCoroutine(EmptyContinuation) +} + +fun main() { + autoreleasepool { + startCoroutine { + autoreleasepool { + suspendHere() + } + } + } + + continuation.resume(Unit) +} diff --git a/kotlin-native/backend.native/tests/compilerChecks/t64.kt b/kotlin-native/backend.native/tests/compilerChecks/t64.kt new file mode 100644 index 00000000000..6ff41a4b873 --- /dev/null +++ b/kotlin-native/backend.native/tests/compilerChecks/t64.kt @@ -0,0 +1,32 @@ +import kotlinx.cinterop.* +import kotlin.coroutines.* + +open class EmptyContinuation(override val context: CoroutineContext = EmptyCoroutineContext) : Continuation { + companion object : EmptyContinuation() + override fun resumeWith(result: Result) { result.getOrThrow() } +} + +lateinit var continuation: Continuation + +suspend fun suspendHere(): Unit = suspendCoroutine { cont -> + continuation = cont +} + + +fun startCoroutine(block: suspend () -> Unit) { + block.startCoroutine(EmptyContinuation) +} + +inline fun myAutoreleasepool(block: () -> T) = autoreleasepool(block) + +fun main() { + autoreleasepool { + startCoroutine { + myAutoreleasepool { + suspendHere() + } + } + } + + continuation.resume(Unit) +} diff --git a/kotlin-native/backend.native/tests/interop/objc/tests/kt50786.kt b/kotlin-native/backend.native/tests/interop/objc/tests/kt50786.kt new file mode 100644 index 00000000000..949e92b8882 --- /dev/null +++ b/kotlin-native/backend.native/tests/interop/objc/tests/kt50786.kt @@ -0,0 +1,34 @@ +package kt50786 + +import kotlinx.cinterop.* +import kotlin.coroutines.* + +open class EmptyContinuation(override val context: CoroutineContext = EmptyCoroutineContext) : Continuation { + companion object : EmptyContinuation() + override fun resumeWith(result: Result) { result.getOrThrow() } +} + +lateinit var continuation: Continuation + +suspend fun suspendHere(): Unit = suspendCoroutine { cont -> + continuation = cont +} + + +fun startCoroutine(block: suspend () -> Unit) { + block.startCoroutine(EmptyContinuation) +} + +@kotlin.test.Test +fun testSafeSuspensionIsAllowedInAutoreleasepool() { + // This test checks that the compiler doesn't prohibit calling suspend functions from `autoreleasepool {}` + // if this call is not actually in the block, but in the local declaration inside it. + // See https://youtrack.jetbrains.com/issue/KT-50786 for more details. + autoreleasepool { + startCoroutine { + suspendHere() + } + } + + continuation.resume(Unit) +}