From dcf7b840828b19dfc905f407eec86c198730d82f Mon Sep 17 00:00:00 2001 From: Marco Pennekamp Date: Tue, 5 Dec 2023 21:16:06 +0100 Subject: [PATCH] [Test] Reset `ApplicationManager.application` when shared application environments are disposed - Shared application environments can be disposed in the middle of test plan execution when the project count reaches zero. So resetting `ApplicationManager` only on "test plan execution finished" is incorrect, because it does not account for that scenario. Instead, we should reset `ApplicationManager` every time the application environment is disposed. - We should also keep the manual reset in `testPlanExecutionFinished` because `disposeApplicationEnvironment` may not need to dispose anything if there isn't a shared application environment, but we should still reset `ApplicationManager` if an unshared application environment was created. - This fixes "Some test disposed, but forgot to clear MockApplication" exceptions which appeared after KT-64099 because it opened up proper disposal of the shared application environment (instead of it being in a state of leakage because not all project disposables were properly disposed). ^KT-63650 ^KT-64099 --- .../cli/jvm/compiler/KotlinCoreEnvironment.kt | 28 +++++++++++++++++++ .../test/ApplicationEnvironmentDisposer.kt | 6 +--- .../test/testFramework/TestApplicationUtil.kt | 7 ++--- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/compiler/cli/cli-base/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinCoreEnvironment.kt b/compiler/cli/cli-base/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinCoreEnvironment.kt index 215f5c6c04f..20a1d0927b3 100644 --- a/compiler/cli/cli-base/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinCoreEnvironment.kt +++ b/compiler/cli/cli-base/src/org/jetbrains/kotlin/cli/jvm/compiler/KotlinCoreEnvironment.kt @@ -16,6 +16,7 @@ import com.intellij.ide.highlighter.JavaFileType import com.intellij.lang.java.JavaParserDefinition import com.intellij.mock.MockProject import com.intellij.openapi.Disposable +import com.intellij.openapi.application.Application import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.application.TransactionGuard import com.intellij.openapi.application.TransactionGuardImpl @@ -594,10 +595,37 @@ class KotlinCoreEnvironment private constructor( val environment = ourApplicationEnvironment ?: return ourApplicationEnvironment = null Disposer.dispose(environment.parentDisposable) + resetApplicationManager(environment.application) ZipHandler.clearFileAccessorCache() } } + /** + * Resets the application managed by [ApplicationManager] to `null`. If [applicationToReset] is specified, [resetApplicationManager] + * will only reset the application if it's the expected one. Otherwise, the application will already have been changed to another + * application. For example, application disposal can trigger one of the disposables registered via + * [ApplicationManager.setApplication], which reset the managed application to the previous application. + */ + @JvmStatic + fun resetApplicationManager(applicationToReset: Application? = null) { + val currentApplication = ApplicationManager.getApplication() ?: return + if (applicationToReset != null && applicationToReset != currentApplication) { + return + } + + try { + val ourApplicationField = ApplicationManager::class.java.getDeclaredField("ourApplication") + ourApplicationField.isAccessible = true + ourApplicationField.set(null, null) + } catch (exception: Exception) { + // Resetting the application manager is not critical in a production context. If the reflective access fails, we shouldn't + // expose the user to the failure. + if (currentApplication.isUnitTestMode) { + throw exception + } + } + } + @JvmStatic fun ProjectEnvironment.configureProjectEnvironment( configuration: CompilerConfiguration, diff --git a/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/ApplicationEnvironmentDisposer.kt b/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/ApplicationEnvironmentDisposer.kt index 09ab01cb4f6..23721d8c7c3 100644 --- a/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/ApplicationEnvironmentDisposer.kt +++ b/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/ApplicationEnvironmentDisposer.kt @@ -6,13 +6,11 @@ package org.jetbrains.kotlin.test import com.intellij.openapi.Disposable -import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.util.Disposer import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment import org.jetbrains.kotlin.test.services.ApplicationDisposableProvider import org.junit.platform.launcher.TestExecutionListener import org.junit.platform.launcher.TestPlan -import java.lang.reflect.Field class ApplicationEnvironmentDisposer : TestExecutionListener { companion object { @@ -22,9 +20,7 @@ class ApplicationEnvironmentDisposer : TestExecutionListener { override fun testPlanExecutionFinished(testPlan: TestPlan) { KotlinCoreEnvironment.disposeApplicationEnvironment() Disposer.dispose(ROOT_DISPOSABLE) - val ourApplicationField: Field = ApplicationManager::class.java.getDeclaredField("ourApplication") - ourApplicationField.isAccessible = true - ourApplicationField.set(null, null) + KotlinCoreEnvironment.resetApplicationManager() } } diff --git a/compiler/tests-common/tests/org/jetbrains/kotlin/test/testFramework/TestApplicationUtil.kt b/compiler/tests-common/tests/org/jetbrains/kotlin/test/testFramework/TestApplicationUtil.kt index f2e57de84d6..135cc1be186 100644 --- a/compiler/tests-common/tests/org/jetbrains/kotlin/test/testFramework/TestApplicationUtil.kt +++ b/compiler/tests-common/tests/org/jetbrains/kotlin/test/testFramework/TestApplicationUtil.kt @@ -6,9 +6,8 @@ package org.jetbrains.kotlin.test.testFramework import com.intellij.openapi.application.Application -import com.intellij.openapi.application.ApplicationManager +import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment import org.jetbrains.kotlin.utils.rethrow -import java.lang.reflect.Field fun resetApplicationToNull(old: Application?) { if (old != null) return @@ -17,9 +16,7 @@ fun resetApplicationToNull(old: Application?) { fun resetApplicationToNull() { try { - val ourApplicationField: Field = ApplicationManager::class.java.getDeclaredField("ourApplication") - ourApplicationField.setAccessible(true) - ourApplicationField.set(null, null) + KotlinCoreEnvironment.resetApplicationManager() } catch (e: Exception) { throw rethrow(e) }