From 42d648f77587f787934feefbe0ce062c6e27a009 Mon Sep 17 00:00:00 2001 From: Sebastian Sellmair Date: Fri, 7 Jul 2023 20:49:19 +0200 Subject: [PATCH] [Gradle] KotlinPluginLifecycle: Implement CoroutineStart and start futures 'Undispatched' by default This API was inspired by kotlinx.coroutines and shall help making Kotlin Gradle Plugins 'Future' type more applicable for situations when the Future is used inside the Configuration Phase ^KT-60158 In Progress --- .idea/dictionaries/sebastiansellmair.xml | 3 +- .../gradle/plugin/KotlinPluginLifecycle.kt | 24 +++- .../plugin/KotlinPluginLifecycleImpl.kt | 12 +- .../plugin/mpp/ProjectMetadataProviderImpl.kt | 26 +++- .../jetbrains/kotlin/gradle/utils/Future.kt | 17 ++- .../kotlin/gradle/unitTests/FutureTest.kt | 14 ++ .../unitTests/KotlinPluginLifecycleTest.kt | 131 +++++++++++++++++- 7 files changed, 203 insertions(+), 24 deletions(-) diff --git a/.idea/dictionaries/sebastiansellmair.xml b/.idea/dictionaries/sebastiansellmair.xml index 75a2abae6dc..6185db2e3b8 100644 --- a/.idea/dictionaries/sebastiansellmair.xml +++ b/.idea/dictionaries/sebastiansellmair.xml @@ -7,6 +7,7 @@ instantiator interops klibrary + undispatched - \ No newline at end of file + diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginLifecycle.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginLifecycle.kt index 3acdf1a8850..82dcf017328 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginLifecycle.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginLifecycle.kt @@ -58,8 +58,11 @@ Util functions * * If the lifecycle already finished and Gradle moved to its execution phase, then the block will be invoked right away. */ -internal fun Project.launch(block: suspend KotlinPluginLifecycle.() -> Unit) { - kotlinPluginLifecycle.launch(block) +internal fun Project.launch( + start: CoroutineStart = CoroutineStart.Default, + block: suspend KotlinPluginLifecycle.() -> Unit, +) { + kotlinPluginLifecycle.launch(start, block) } /** @@ -289,6 +292,18 @@ internal interface KotlinPluginLifecycle { } } + enum class CoroutineStart { + /** + * Puts a coroutine at the end of the current execution queue + */ + Default, + + /** + * Immediately executes the coroutine until its first suspension point in the current thread + */ + Undispatched + } + sealed class ProjectConfigurationResult { object Success : ProjectConfigurationResult() data class Failure(val failures: List) : ProjectConfigurationResult() @@ -302,7 +317,10 @@ internal interface KotlinPluginLifecycle { val isFinished: Boolean - fun launch(block: suspend KotlinPluginLifecycle.() -> Unit) + fun launch( + start: CoroutineStart = CoroutineStart.Default, + block: suspend KotlinPluginLifecycle.() -> Unit, + ) suspend fun await(stage: Stage) diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginLifecycleImpl.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginLifecycleImpl.kt index f2863a20c37..d0de393a0b4 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginLifecycleImpl.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginLifecycleImpl.kt @@ -6,6 +6,8 @@ package org.jetbrains.kotlin.gradle.plugin import org.gradle.api.Project +import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle.CoroutineStart.Default +import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle.CoroutineStart.Undispatched import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle.ProjectConfigurationResult import org.jetbrains.kotlin.gradle.utils.CompletableFuture import org.jetbrains.kotlin.gradle.utils.failures @@ -151,7 +153,10 @@ internal class KotlinPluginLifecycleImpl(override val project: Project) : Kotlin } } - override fun launch(block: suspend KotlinPluginLifecycle.() -> Unit) { + override fun launch( + start: KotlinPluginLifecycle.CoroutineStart, + block: suspend KotlinPluginLifecycle.() -> Unit, + ) { val lifecycle = this val coroutine = block.createCoroutine(this, object : Continuation { @@ -161,8 +166,9 @@ internal class KotlinPluginLifecycleImpl(override val project: Project) : Kotlin override fun resumeWith(result: Result) = result.getOrThrow() }) - enqueue(stage) { - coroutine.resume(Unit) + when (start) { + Default -> enqueue(stage) { coroutine.resume(Unit) } + Undispatched -> coroutine.resume(Unit) } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/ProjectMetadataProviderImpl.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/ProjectMetadataProviderImpl.kt index c477a4195c9..e7dd4d9a123 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/ProjectMetadataProviderImpl.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/ProjectMetadataProviderImpl.kt @@ -10,6 +10,7 @@ import org.gradle.api.file.FileCollection import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension import org.jetbrains.kotlin.gradle.dsl.multiplatformExtensionOrNull import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle +import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle.Stage.AfterEvaluateBuildscript import org.jetbrains.kotlin.gradle.plugin.KotlinSourceSet import org.jetbrains.kotlin.gradle.plugin.await import org.jetbrains.kotlin.gradle.plugin.mpp.MetadataDependencyResolution.ChooseVisibleSourceSets.MetadataProvider.ProjectMetadataProvider @@ -19,23 +20,23 @@ import org.jetbrains.kotlin.gradle.targets.native.internal.* private typealias SourceSetName = String internal fun ProjectMetadataProvider( - sourceSetMetadataOutputs: Map + sourceSetMetadataOutputs: Map, ): ProjectMetadataProvider { return ProjectMetadataProviderImpl(sourceSetMetadataOutputs) } internal class SourceSetMetadataOutputs( val metadata: FileCollection?, - val cinterop: CInterop? + val cinterop: CInterop?, ) { class CInterop( val forCli: FileCollection, - val forIde: FileCollection + val forIde: FileCollection, ) } private class ProjectMetadataProviderImpl( - private val sourceSetMetadataOutputs: Map + private val sourceSetMetadataOutputs: Map, ) : ProjectMetadataProvider() { override fun getSourceSetCompiledMetadata(sourceSetName: String): FileCollection? { @@ -55,6 +56,19 @@ private class ProjectMetadataProviderImpl( } internal suspend fun Project.collectSourceSetMetadataOutputs(): Map { + /* + Usually we can safely access the kotlin project extension inside a coroutine, as the Kotlin Gradle Plugin is + the only entity that could launch coroutines (hence the extension being available). + + However, this code is crossing project boundaries here: + There is _some_ Kotlin Gradle plugin that requests the 'ProjectData' being collected for + *all* projects (breaking project isolation). + + Therefore, it might happen that the Kotlin Plugin was not even applied at this point, when this + coroutine starts executing. We therefore await the wait for after the buildscript was evaluated to check + if the multiplatformExtension is present. + */ + AfterEvaluateBuildscript.await() val multiplatformExtension = multiplatformExtensionOrNull ?: return emptyMap() val sourceSetMetadata = multiplatformExtension.sourceSetsMetadataOutputs() @@ -83,7 +97,7 @@ private suspend fun KotlinMultiplatformExtension.sourceSetsMetadataOutputs(): Ma } private suspend fun KotlinMultiplatformExtension.cInteropMetadataOfSourceSets( - sourceSets: Iterable + sourceSets: Iterable, ): Map { val taskForCLI = project.commonizeCInteropTask() ?: return emptyMap() val taskForIde = project.copyCommonizeCInteropForIdeTask() ?: return emptyMap() @@ -95,4 +109,4 @@ private suspend fun KotlinMultiplatformExtension.cInteropMetadataOfSourceSets( forIde = taskForIde.get().commonizedOutputLibraries(dependent) ) } -} \ No newline at end of file +} diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/utils/Future.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/utils/Future.kt index aa29237c658..20858e94b0c 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/utils/Future.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/utils/Future.kt @@ -8,6 +8,7 @@ package org.jetbrains.kotlin.gradle.utils import org.gradle.api.Project import org.jetbrains.kotlin.gradle.plugin.HasProject import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle +import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle.CoroutineStart.Undispatched import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle.IllegalLifecycleException import org.jetbrains.kotlin.gradle.plugin.kotlinPluginLifecycle import org.jetbrains.kotlin.tooling.core.ExtrasLazyProperty @@ -75,7 +76,10 @@ internal inline fun futureExtension( } } -internal fun Project.future(block: suspend Project.() -> T): Future = kotlinPluginLifecycle.future { block() } +internal fun Project.future( + start: KotlinPluginLifecycle.CoroutineStart = Undispatched, + block: suspend Project.() -> T, +): Future = kotlinPluginLifecycle.future(start) { block() } internal val Future.lenient: LenientFuture get() = LenientFutureImpl(this) @@ -87,11 +91,14 @@ internal val Future.lenient: LenientFuture get() = LenientFutureImpl(t * * basically creating a future, which is launched lazily */ -internal fun Project.lazyFuture(block: suspend Project.() -> T): Future = LazyFutureImpl(lazy { future(block) }) +internal fun Project.lazyFuture(block: suspend Project.() -> T): Future = LazyFutureImpl(lazy { future { block() } }) -internal fun KotlinPluginLifecycle.future(block: suspend () -> T): Future { +internal fun KotlinPluginLifecycle.future( + start: KotlinPluginLifecycle.CoroutineStart = Undispatched, + block: suspend () -> T, +): Future { return FutureImpl(lifecycle = this).also { future -> - launch { future.completeWith(runCatching { block() }) } + launch(start) { future.completeWith(runCatching { block() }) } } } @@ -267,4 +274,4 @@ private class Completable( waitingContinuations.add(continuation) }.getOrThrow() } -} \ No newline at end of file +} diff --git a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/FutureTest.kt b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/FutureTest.kt index 144d4130856..5c029785574 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/FutureTest.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/FutureTest.kt @@ -10,6 +10,7 @@ package org.jetbrains.kotlin.gradle.unitTests import org.jetbrains.kotlin.gradle.idea.testFixtures.utils.deserialize import org.jetbrains.kotlin.gradle.idea.testFixtures.utils.serialize import org.jetbrains.kotlin.gradle.plugin.* +import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle.CoroutineStart import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle.IllegalLifecycleException import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle.Stage.FinaliseDsl import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle.Stage.ReadyForExecution @@ -150,4 +151,17 @@ class FutureTest { future.await() assertEquals(1, mappedFuture.getOrThrow()) } + + @Test + fun `test - future is launched 'Undispatched' by default`() = project.runLifecycleAwareTest { + val future = project.future { } + future.getOrThrow() + } + + @Test + fun `test - future with CoroutineStart 'Default'`() = project.runLifecycleAwareTest { + val future = project.future(CoroutineStart.Default) {} + assertFailsWith { future.getOrThrow() } + future.await() + } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/KotlinPluginLifecycleTest.kt b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/KotlinPluginLifecycleTest.kt index 261f9d654c1..10a8bf92a59 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/KotlinPluginLifecycleTest.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/KotlinPluginLifecycleTest.kt @@ -8,12 +8,10 @@ package org.jetbrains.kotlin.gradle.unitTests import org.gradle.api.ProjectConfigurationException -import org.gradle.internal.impldep.org.testng.TestException import org.gradle.testfixtures.ProjectBuilder import org.jetbrains.kotlin.gradle.plugin.* -import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle.IllegalLifecycleException +import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle.* import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle.ProjectConfigurationResult.Failure -import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle.Stage import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle.Stage.* import org.jetbrains.kotlin.gradle.util.applyMultiplatformPlugin import org.jetbrains.kotlin.gradle.util.buildProjectWithMPP @@ -561,12 +559,133 @@ class KotlinPluginLifecycleTest { fun `test - runLifecycleAwareTest - in a project with lifecycle`() { val project = ProjectBuilder.builder().build() project.applyMultiplatformPlugin() - project.runLifecycleAwareTest{} + project.runLifecycleAwareTest {} } @Test fun `test - runLifecycleAwareTest - in a project without lifecycle`() { val project = ProjectBuilder.builder().build() - assertFailsWith { project.runLifecycleAwareTest{} } + assertFailsWith { project.runLifecycleAwareTest {} } } -} \ No newline at end of file + + @Test + fun `test - launch with CoroutineStart - Default`() = project.runLifecycleAwareTest { + val rootCoroutineExecuted = AtomicBoolean() + val coroutineAExecuted = AtomicBoolean() + val coroutineBExecuted = AtomicBoolean() + + launch(CoroutineStart.Default) coroutineA@{ + /* + This 'coroutine A' is launched using 'Default' which means that it will + be 'dispatched' by being placed at the back of the queue. + + The currently executing 'root coroutine' therefore will finish. + Only if the previous queue is empty, this coroutine will start execution + */ + assertTrue( + rootCoroutineExecuted.get(), + "Expected 'root coroutine' to have executed" + ) + + + assertFalse( + coroutineBExecuted.get(), + "Expected 'coroutineA' being executed before 'coroutineB'" + ) + + assertFalse(coroutineAExecuted.getAndSet(true)) + } + + launch(CoroutineStart.Default) coroutineB@{ + /* + See code comment above: This 'coroutineB' is put at the end of the current execution queue + */ + assertTrue( + rootCoroutineExecuted.get(), + "Expected 'root coroutine' to have executed" + ) + assertTrue( + coroutineAExecuted.get(), + "Expected 'coroutineA' being executed before 'coroutineB'" + ) + assertFalse(coroutineBExecuted.getAndSet(true)) + } + + assertFalse(rootCoroutineExecuted.getAndSet(true)) + + assertFalse( + coroutineAExecuted.get(), + "Expected 'coroutineA' to not be executed yet" + ) + + assertFalse( + coroutineBExecuted.get(), + "Expected 'coroutineB' to not be executed yet" + ) + + launch(CoroutineStart.Default) { + assertTrue( + coroutineAExecuted.get(), + "Expected 'coroutineA' to be executed" + ) + + assertTrue( + coroutineBExecuted.get(), + "Expected 'coroutineB' to be executed" + ) + } + } + + @Test + fun `test - launch with CoroutineStart - Undispatched`() = project.runLifecycleAwareTest { + val rootCoroutineFinished = AtomicBoolean() + val coroutineAExecuted = AtomicBoolean() + val coroutineBExecuted = AtomicBoolean() + + launch(CoroutineStart.Undispatched) coroutineA@{ + /* + Undispatched will start executing the coroutine 'inline' and will + only suspend once the first 'suspension' point is hit. + This means, that at this point, the 'root coroutine' had no chance to finsih yet. + */ + assertFalse( + rootCoroutineFinished.get(), + "Expected 'root coroutine' to not have finished yet" + ) + + + assertFalse( + coroutineBExecuted.get(), + "Expected 'coroutineA' to be executed before 'coroutineB'" + ) + + assertFalse(coroutineAExecuted.getAndSet(true)) + } + + assertTrue( + coroutineAExecuted.get(), + "Expected 'coroutineA' to be executed 'inline'" + ) + + launch(CoroutineStart.Undispatched) coroutineB@{ + /* + See comment in coroutineA + */ + assertFalse( + rootCoroutineFinished.get(), + "Expected 'root coroutine' to not have finished yet" + ) + + assertTrue( + coroutineAExecuted.get(), + "Expected 'coroutineA' to be executed before 'coroutineB'" + ) + assertFalse(coroutineBExecuted.getAndSet(true)) + } + + assertFalse(rootCoroutineFinished.getAndSet(true)) + assertTrue(coroutineAExecuted.get()) + assertTrue(coroutineBExecuted.get()) + } + +}