From 64a4d09bb3b685311bd2f08ba5669c18dec325c3 Mon Sep 17 00:00:00 2001 From: Mikhail Bogdanov Date: Thu, 19 Mar 2020 08:36:40 +0100 Subject: [PATCH] Add basic synchronization workaround for scratch --- .../kotlin/idea/scratch/ScratchExecutor.kt | 2 ++ .../scratch/repl/KtScratchReplExecutor.kt | 19 +++++++++++++++++-- idea/scripting-support/build.gradle.kts | 2 +- .../scratch/AbstractScratchRunActionTest.kt | 11 +++++++++-- 4 files changed, 29 insertions(+), 5 deletions(-) diff --git a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/scratch/ScratchExecutor.kt b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/scratch/ScratchExecutor.kt index 5c04e9ba274..56bac48026b 100644 --- a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/scratch/ScratchExecutor.kt +++ b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/scratch/ScratchExecutor.kt @@ -17,6 +17,7 @@ package org.jetbrains.kotlin.idea.scratch import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.application.invokeAndWaitIfNeeded import com.intellij.openapi.application.runReadAction import com.intellij.openapi.diagnostic.ControlFlowException import com.intellij.openapi.editor.EditorFactory @@ -188,6 +189,7 @@ abstract class SequentialScratchExecutor(file: ScratchFile) : ScratchExecutor(fi stopExecution { lock.release() } + // blocking UI thread!? check(lock.tryAcquire(2, TimeUnit.SECONDS)) { "Couldn't stop REPL process in 2 seconds" } diff --git a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/scratch/repl/KtScratchReplExecutor.kt b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/scratch/repl/KtScratchReplExecutor.kt index 057ed98242d..6d3b2c4ca90 100644 --- a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/scratch/repl/KtScratchReplExecutor.kt +++ b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/scratch/repl/KtScratchReplExecutor.kt @@ -29,6 +29,8 @@ import javax.xml.parsers.DocumentBuilderFactory class KtScratchReplExecutor(file: ScratchFile) : SequentialScratchExecutor(file) { private val history: ReplHistory = ReplHistory() + + @Volatile private var osProcessHandler: OSProcessHandler? = null override fun startExecution() { @@ -65,6 +67,8 @@ class KtScratchReplExecutor(file: ScratchFile) : SequentialScratchExecutor(file) } } + // There should be some kind of more wise synchronization cause this method is called from non-UI thread (process handler thread) + // and actually there could be side effects in handlers private fun clearState() { history.clear() osProcessHandler = null @@ -145,9 +149,20 @@ class KtScratchReplExecutor(file: ScratchFile) : SequentialScratchExecutor(file) } override fun notifyProcessTerminated(exitCode: Int) { - super.notifyProcessTerminated(exitCode) - + // Do state cleaning before notification otherwise KtScratchFileEditorWithPreview.dispose + // would try to stop process again (after stop in tests 'stopReplProcess`) + // via `stopExecution` (because handler is not null) with next exception: + // + // Caused by: com.intellij.testFramework.LoggedErrorProcessor$TestLoggerAssertionError: The pipe is being closed + // at com.intellij.testFramework.LoggedErrorProcessor.processError(LoggedErrorProcessor.java:66) + // at com.intellij.testFramework.TestLogger.error(TestLogger.java:40) + // at com.intellij.openapi.diagnostic.Logger.error(Logger.java:170) + // at org.jetbrains.kotlin.idea.scratch.ScratchExecutor.errorOccurs(ScratchExecutor.kt:50) + // at org.jetbrains.kotlin.idea.scratch.repl.KtScratchReplExecutor.stopExecution(KtScratchReplExecutor.kt:61) + // at org.jetbrains.kotlin.idea.scratch.SequentialScratchExecutor.stopExecution$default(ScratchExecutor.kt:90) clearState() + + super.notifyProcessTerminated(exitCode) } private fun strToSource(s: String, encoding: Charset = Charsets.UTF_8) = InputSource(ByteArrayInputStream(s.toByteArray(encoding))) diff --git a/idea/scripting-support/build.gradle.kts b/idea/scripting-support/build.gradle.kts index c278b254b2c..8ea0e1daebb 100644 --- a/idea/scripting-support/build.gradle.kts +++ b/idea/scripting-support/build.gradle.kts @@ -45,7 +45,7 @@ sourceSets { "test" { projectDefault() } } -projectTest { +projectTest(parallel = true) { dependsOn(":dist") workingDir = rootDir } diff --git a/idea/scripting-support/test/org/jetbrains/kotlin/idea/scratch/AbstractScratchRunActionTest.kt b/idea/scripting-support/test/org/jetbrains/kotlin/idea/scratch/AbstractScratchRunActionTest.kt index 296ea845d07..5a6f7addea6 100644 --- a/idea/scripting-support/test/org/jetbrains/kotlin/idea/scratch/AbstractScratchRunActionTest.kt +++ b/idea/scripting-support/test/org/jetbrains/kotlin/idea/scratch/AbstractScratchRunActionTest.kt @@ -24,6 +24,7 @@ import org.jetbrains.kotlin.codegen.forTestCompile.ForTestCompileRuntime import org.jetbrains.kotlin.idea.KotlinLanguage import org.jetbrains.kotlin.idea.actions.KOTLIN_WORKSHEET_EXTENSION import org.jetbrains.kotlin.idea.core.script.ScriptConfigurationManager +import org.jetbrains.kotlin.idea.debugger.coroutine.util.logger import org.jetbrains.kotlin.idea.highlighter.KotlinHighlightingUtil import org.jetbrains.kotlin.idea.scratch.actions.ClearScratchAction import org.jetbrains.kotlin.idea.scratch.actions.RunScratchAction @@ -99,7 +100,7 @@ abstract class AbstractScratchRunActionTest : FileEditorManagerTestCase() { ) } - val outputDir = createTempDir(dirName) + val outputDir = FileUtil.createTempDirectory(dirName, "") if (javaFiles.isNotEmpty()) { val options = listOf("-d", outputDir.path) @@ -259,7 +260,11 @@ abstract class AbstractScratchRunActionTest : FileEditorManagerTestCase() { val start = System.currentTimeMillis() // wait until output is displayed in editor or for 1 minute - while (ScratchCompilationSupport.isAnyInProgress() && (System.currentTimeMillis() - start) < 60000) { + while (ScratchCompilationSupport.isAnyInProgress()) { + if ((System.currentTimeMillis() - start) > TIME_OUT) { + LOG.warn("Waiting timeout $TIME_OUT ms is exceed") + break + } Thread.sleep(100) } @@ -323,6 +328,8 @@ abstract class AbstractScratchRunActionTest : FileEditorManagerTestCase() { } companion object { + private const val TIME_OUT = 60000 // 1 min + private val INSTANCE_WITH_KOTLIN_TEST = object : KotlinWithJdkAndRuntimeLightProjectDescriptor( arrayListOf( ForTestCompileRuntime.runtimeJarForTests(),