From 46894da981735cf3cd3086ff3403af67a293c0fc Mon Sep 17 00:00:00 2001 From: Ilya Chernikov Date: Thu, 9 Feb 2017 16:36:18 +0100 Subject: [PATCH] Get rid of daemon-client dependency on openapi, making others "provided" (cherry picked from commit 175d74c) --- .../daemon/daemon-client/daemon-client.iml | 11 +-- .../CompilerCallbackServicesFacadeServer.kt | 11 ++- .../daemon/client/KotlinRemoteReplClient.kt | 24 ++--- .../kotlin/daemon/CompilerDaemonTest.kt | 97 ++++++++----------- .../KotlinJsr223JvmScriptEngine4Idea.kt | 7 +- ...nJsr223StandardScriptEngineFactory4Idea.kt | 5 +- ...otlinJsr223JvmDaemonCompileScriptEngine.kt | 3 +- 7 files changed, 67 insertions(+), 91 deletions(-) diff --git a/compiler/daemon/daemon-client/daemon-client.iml b/compiler/daemon/daemon-client/daemon-client.iml index c0335b99f66..8bf6dd7157f 100644 --- a/compiler/daemon/daemon-client/daemon-client.iml +++ b/compiler/daemon/daemon-client/daemon-client.iml @@ -7,11 +7,10 @@ - - - - - - + + + + + \ No newline at end of file diff --git a/compiler/daemon/daemon-client/src/org/jetbrains/kotlin/daemon/client/CompilerCallbackServicesFacadeServer.kt b/compiler/daemon/daemon-client/src/org/jetbrains/kotlin/daemon/client/CompilerCallbackServicesFacadeServer.kt index e6da94f69c2..58859d69095 100644 --- a/compiler/daemon/daemon-client/src/org/jetbrains/kotlin/daemon/client/CompilerCallbackServicesFacadeServer.kt +++ b/compiler/daemon/daemon-client/src/org/jetbrains/kotlin/daemon/client/CompilerCallbackServicesFacadeServer.kt @@ -16,7 +16,6 @@ package org.jetbrains.kotlin.daemon.client -import com.intellij.openapi.progress.ProcessCanceledException import org.jetbrains.kotlin.daemon.common.CompilerCallbackServicesFacade import org.jetbrains.kotlin.daemon.common.LoopbackNetworkInterface import org.jetbrains.kotlin.daemon.common.RmiFriendlyCompilationCanceledException @@ -27,7 +26,9 @@ import org.jetbrains.kotlin.load.kotlin.incremental.components.IncrementalCompil import org.jetbrains.kotlin.load.kotlin.incremental.components.JvmPackagePartProto import org.jetbrains.kotlin.modules.TargetId import org.jetbrains.kotlin.progress.CompilationCanceledStatus +import org.jetbrains.kotlin.utils.rethrow import java.rmi.server.UnicastRemoteObject +import kotlin.reflect.full.allSuperclasses open class CompilerCallbackServicesFacadeServer( @@ -83,10 +84,12 @@ open class CompilerCallbackServicesFacadeServer( try { compilationCanceledStatus!!.checkCanceled() } - catch (e: ProcessCanceledException) { + catch (e: Exception) { // avoid passing exceptions that may have different serialVersionUID on across rmi border - // TODO: doublecheck whether we need to distinguish different cancellation exceptions - throw RmiFriendlyCompilationCanceledException() + // removing dependency from openapi (this is obsolete part anyway, and will be removed soon) + if ((e::class.allSuperclasses + e::class).any { it.qualifiedName == "com.intellij.openapi.progress.ProcessCanceledException" }) + throw RmiFriendlyCompilationCanceledException() + else throw e } } } diff --git a/compiler/daemon/daemon-client/src/org/jetbrains/kotlin/daemon/client/KotlinRemoteReplClient.kt b/compiler/daemon/daemon-client/src/org/jetbrains/kotlin/daemon/client/KotlinRemoteReplClient.kt index fe6516d4a33..ddefdb28325 100644 --- a/compiler/daemon/daemon-client/src/org/jetbrains/kotlin/daemon/client/KotlinRemoteReplClient.kt +++ b/compiler/daemon/daemon-client/src/org/jetbrains/kotlin/daemon/client/KotlinRemoteReplClient.kt @@ -16,8 +16,6 @@ package org.jetbrains.kotlin.daemon.client -import com.intellij.openapi.Disposable -import com.intellij.openapi.util.Disposer import org.jetbrains.kotlin.cli.common.repl.* import org.jetbrains.kotlin.daemon.common.CompileService import org.jetbrains.kotlin.daemon.common.RemoteOperationsTracer @@ -29,7 +27,6 @@ import java.io.OutputStream // TODO: reduce number of ports used then SOCKET_ANY_FREE_PORT is passed (same problem with other calls) open class KotlinRemoteReplClientBase( - disposable: Disposable, protected val compileService: CompileService, clientAliveFlagFile: File?, targetPlatform: CompileService.TargetPlatform, @@ -60,20 +57,18 @@ open class KotlinRemoteReplClientBase( operationsTracer ).get() - init { - Disposer.register(disposable, Disposable { - try { - compileService.releaseReplSession(sessionId) - } - catch (ex: java.rmi.RemoteException) { - // assuming that communication failed and daemon most likely is already down - } - }) + // dispose should be called at the end of the repl lifetime to free daemon repl session and appropriate resources + open fun dispose() { + try { + compileService.releaseReplSession(sessionId) + } + catch (ex: java.rmi.RemoteException) { + // assuming that communication failed and daemon most likely is already down + } } } class KotlinRemoteReplCompiler( - disposable: Disposable, compileService: CompileService, clientAliveFlagFile: File?, targetPlatform: CompileService.TargetPlatform, @@ -83,7 +78,6 @@ class KotlinRemoteReplCompiler( port: Int = SOCKET_ANY_FREE_PORT, operationsTracer: RemoteOperationsTracer? = null ) : KotlinRemoteReplClientBase( - disposable = disposable, compileService = compileService, clientAliveFlagFile = clientAliveFlagFile, targetPlatform = targetPlatform, @@ -114,7 +108,6 @@ class KotlinRemoteReplCompiler( // TODO: consider removing daemon eval completely - it is not required now and has questionable security. This will simplify daemon interface as well class KotlinRemoteReplEvaluator( - disposable: Disposable, compileService: CompileService, clientAliveFlagFile: File?, targetPlatform: CompileService.TargetPlatform, @@ -129,7 +122,6 @@ class KotlinRemoteReplEvaluator( port: Int = SOCKET_ANY_FREE_PORT, operationsTracer: RemoteOperationsTracer? = null ) : KotlinRemoteReplClientBase( - disposable = disposable, compileService = compileService, clientAliveFlagFile = clientAliveFlagFile, targetPlatform = targetPlatform, diff --git a/compiler/tests/org/jetbrains/kotlin/daemon/CompilerDaemonTest.kt b/compiler/tests/org/jetbrains/kotlin/daemon/CompilerDaemonTest.kt index 0693affc71c..1a1befdcfe6 100644 --- a/compiler/tests/org/jetbrains/kotlin/daemon/CompilerDaemonTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/daemon/CompilerDaemonTest.kt @@ -16,8 +16,6 @@ package org.jetbrains.kotlin.daemon -import com.intellij.openapi.Disposable -import com.intellij.openapi.util.Disposer import junit.framework.TestCase import org.jetbrains.kotlin.cli.AbstractCliTest import org.jetbrains.kotlin.cli.common.repl.* @@ -480,32 +478,30 @@ class CompilerDaemonTest : KotlinIntegrationTestBase() { fun testDaemonReplLocalEvalNoParams() { withDaemon { daemon -> - withDisposable { disposable -> - val repl = KotlinRemoteReplCompiler(disposable, daemon!!, null, CompileService.TargetPlatform.JVM, - classpathFromClassloader(), - ScriptWithNoParam::class.qualifiedName!!, - System.err) + val repl = KotlinRemoteReplCompiler(daemon, null, CompileService.TargetPlatform.JVM, + classpathFromClassloader(), + ScriptWithNoParam::class.qualifiedName!!, + System.err) - val localEvaluator = GenericReplEvaluator(emptyList(), Thread.currentThread().contextClassLoader) + val localEvaluator = GenericReplEvaluator(emptyList(), Thread.currentThread().contextClassLoader) - doReplTestWithLocalEval(repl, localEvaluator) - } + doReplTestWithLocalEval(repl, localEvaluator) + repl.dispose() } } fun testDaemonReplLocalEvalStandardTemplate() { withDaemon { daemon -> - withDisposable { disposable -> - val repl = KotlinRemoteReplCompiler(disposable, daemon!!, null, CompileService.TargetPlatform.JVM, - classpathFromClassloader(), - "kotlin.script.templates.standard.ScriptTemplateWithArgs", - System.err) + val repl = KotlinRemoteReplCompiler(daemon, null, CompileService.TargetPlatform.JVM, + classpathFromClassloader(), + "kotlin.script.templates.standard.ScriptTemplateWithArgs", + System.err) - val localEvaluator = GenericReplEvaluator(emptyList(), Thread.currentThread().contextClassLoader, - ScriptArgsWithTypes(arrayOf(emptyArray()), arrayOf(Array::class))) + val localEvaluator = GenericReplEvaluator(emptyList(), Thread.currentThread().contextClassLoader, + ScriptArgsWithTypes(arrayOf(emptyArray()), arrayOf(Array::class))) - doReplTestWithLocalEval(repl, localEvaluator) - } + doReplTestWithLocalEval(repl, localEvaluator) + repl.dispose() } } @@ -538,36 +534,34 @@ class CompilerDaemonTest : KotlinIntegrationTestBase() { fun testDaemonReplRemoteEval() { withDaemon { daemon -> - withDisposable { disposable -> + val evalOut = ByteArrayOutputStream() + val evalErr = ByteArrayOutputStream() + val evalIn = ByteArrayInputStream("42\n".toByteArray()) - val evalOut = ByteArrayOutputStream() - val evalErr = ByteArrayOutputStream() - val evalIn = ByteArrayInputStream("42\n".toByteArray()) + val repl = KotlinRemoteReplEvaluator(daemon, null, CompileService.TargetPlatform.JVM, + listOf(File(KotlinIntegrationTestBase.getCompilerLib(), "kotlin-runtime.jar")), + "kotlin.script.templates.standard.ScriptTemplateWithArgs", + arrayOf(emptyArray()), + arrayOf(Array::class.java), + System.err, evalOut, evalErr, evalIn) - val repl = KotlinRemoteReplEvaluator(disposable, daemon!!, null, CompileService.TargetPlatform.JVM, - listOf(File(KotlinIntegrationTestBase.getCompilerLib(), "kotlin-runtime.jar")), - "kotlin.script.templates.standard.ScriptTemplateWithArgs", - arrayOf(emptyArray()), - arrayOf(Array::class.java), - System.err, evalOut, evalErr, evalIn) + val res0 = repl.check(ReplCodeLine(0, "val x =")) + TestCase.assertTrue("Unexpected check results: $res0", res0 is ReplCheckResult.Incomplete) - val res0 = repl.check(ReplCodeLine(0, "val x =")) - TestCase.assertTrue("Unexpected check results: $res0", res0 is ReplCheckResult.Incomplete) + val codeLine1 = ReplCodeLine(1, "val x = 5") + val res1 = repl.compileAndEval(codeLine1, verifyHistory = emptyList()) + val res1e = res1 as? ReplEvalResult.UnitResult + TestCase.assertNotNull("Unexpected eval result: $res1", res1e) - val codeLine1 = ReplCodeLine(1, "val x = 5") - val res1 = repl.compileAndEval(codeLine1, verifyHistory = emptyList()) - val res1e = res1 as? ReplEvalResult.UnitResult - TestCase.assertNotNull("Unexpected eval result: $res1", res1e) + val codeLine2 = ReplCodeLine(2, "x + 2") + val res2x = repl.compileAndEval(codeLine2, verifyHistory = listOf(codeLine2)) + TestCase.assertNotNull("Unexpected compile result: $res2x", res2x as? ReplEvalResult.HistoryMismatch) - val codeLine2 = ReplCodeLine(2, "x + 2") - val res2x = repl.compileAndEval(codeLine2, verifyHistory = listOf(codeLine2)) - TestCase.assertNotNull("Unexpected compile result: $res2x", res2x as? ReplEvalResult.HistoryMismatch) - - val res2 = repl.compileAndEval(codeLine2, verifyHistory = listOf(codeLine1)) - val res2e = res2 as? ReplEvalResult.ValueResult - TestCase.assertNotNull("Unexpected eval result: $res2", res2e) - TestCase.assertEquals(7, res2e!!.value) - } + val res2 = repl.compileAndEval(codeLine2, verifyHistory = listOf(codeLine1)) + val res2e = res2 as? ReplEvalResult.ValueResult + TestCase.assertNotNull("Unexpected eval result: $res2", res2e) + TestCase.assertEquals(7, res2e!!.value) + repl.dispose() } } @@ -584,8 +578,7 @@ class CompilerDaemonTest : KotlinIntegrationTestBase() { val daemon = KotlinCompilerClient.connectToCompileService(compilerId, flagFile, daemonJVMOptions, daemonOptions, DaemonReportingTargets(out = System.err), autostart = true) assertNotNull("failed to connect daemon", daemon) - val disposable = Disposer.newDisposable() - val repl = KotlinRemoteReplCompiler(disposable, daemon!!, null, CompileService.TargetPlatform.JVM, + val repl = KotlinRemoteReplCompiler(daemon!!, null, CompileService.TargetPlatform.JVM, classpathFromClassloader(), ScriptWithNoParam::class.qualifiedName!!, System.err) @@ -604,7 +597,7 @@ class CompilerDaemonTest : KotlinIntegrationTestBase() { if (logFile.isLogContainsSequence("Idle timeout exceeded 1s")) break Thread.sleep(200) } - Disposer.dispose(disposable) + repl.dispose() Thread.sleep(200) logFile.assertLogContainsSequence("Idle timeout exceeded 1s", @@ -688,16 +681,6 @@ internal inline fun withFlagFile(prefix: String, suffix: String? = null, body: ( } } -internal inline fun withDisposable(body: (Disposable) -> Unit) { - val disposable = Disposer.newDisposable() - try { - body(disposable) - } - finally { - Disposer.dispose(disposable) - } -} - // java.util.Logger used in the daemon silently forgets to log into a file specified in the config on Windows, // if file path is given in windows form (using backslash as a separator); the reason is unknown // this function makes a path with forward slashed, that works on windows too diff --git a/idea/idea-repl/src/org/jetbrains/kotlin/jsr223/KotlinJsr223JvmScriptEngine4Idea.kt b/idea/idea-repl/src/org/jetbrains/kotlin/jsr223/KotlinJsr223JvmScriptEngine4Idea.kt index 2eb250d2529..438df3332d6 100644 --- a/idea/idea-repl/src/org/jetbrains/kotlin/jsr223/KotlinJsr223JvmScriptEngine4Idea.kt +++ b/idea/idea-repl/src/org/jetbrains/kotlin/jsr223/KotlinJsr223JvmScriptEngine4Idea.kt @@ -16,7 +16,6 @@ package org.jetbrains.kotlin.jsr223 -import com.intellij.openapi.Disposable import org.jetbrains.kotlin.cli.common.repl.* import org.jetbrains.kotlin.daemon.client.DaemonReportMessage import org.jetbrains.kotlin.daemon.client.DaemonReportingTargets @@ -30,8 +29,9 @@ import javax.script.ScriptEngineFactory import javax.script.ScriptException import kotlin.reflect.KClass +// TODO: need to manage resources here, i.e. call replCompiler.dispose when engine is collected + class KotlinJsr223JvmScriptEngine4Idea( - disposable: Disposable, factory: ScriptEngineFactory, templateClasspath: List, templateClassName: String, @@ -54,8 +54,7 @@ class KotlinJsr223JvmScriptEngine4Idea( override val replCompiler: ReplCompiler by lazy { daemon.let { - KotlinRemoteReplCompiler(disposable, - it, + KotlinRemoteReplCompiler(it, makeAutodeletingFlagFile("idea-jsr223-repl-session"), CompileService.TargetPlatform.JVM, templateClasspath, diff --git a/idea/idea-repl/src/org/jetbrains/kotlin/jsr223/KotlinJsr223StandardScriptEngineFactory4Idea.kt b/idea/idea-repl/src/org/jetbrains/kotlin/jsr223/KotlinJsr223StandardScriptEngineFactory4Idea.kt index 38180f5983b..12c5d089b67 100644 --- a/idea/idea-repl/src/org/jetbrains/kotlin/jsr223/KotlinJsr223StandardScriptEngineFactory4Idea.kt +++ b/idea/idea-repl/src/org/jetbrains/kotlin/jsr223/KotlinJsr223StandardScriptEngineFactory4Idea.kt @@ -16,11 +16,11 @@ package org.jetbrains.kotlin.jsr223 -import com.intellij.openapi.util.Disposer import org.jetbrains.kotlin.cli.common.repl.KotlinJsr223JvmScriptEngineFactoryBase import org.jetbrains.kotlin.cli.common.repl.ScriptArgsWithTypes import org.jetbrains.kotlin.utils.PathUtil -import org.jetbrains.kotlin.utils.PathUtil.* +import org.jetbrains.kotlin.utils.PathUtil.KOTLIN_JAVA_SCRIPT_RUNTIME_JAR +import org.jetbrains.kotlin.utils.PathUtil.KOTLIN_JAVA_STDLIB_JAR import java.io.File import java.io.FileNotFoundException import java.net.URL @@ -33,7 +33,6 @@ class KotlinJsr223StandardScriptEngineFactory4Idea : KotlinJsr223JvmScriptEngine override fun getScriptEngine(): ScriptEngine = KotlinJsr223JvmScriptEngine4Idea( - Disposer.newDisposable(), this, scriptCompilationClasspathFromContext(Thread.currentThread().contextClassLoader), "kotlin.script.templates.standard.ScriptTemplateWithBindings", diff --git a/libraries/tools/kotlin-script-util/src/main/kotlin/org/jetbrains/kotlin/script/jsr223/KotlinJsr223JvmDaemonCompileScriptEngine.kt b/libraries/tools/kotlin-script-util/src/main/kotlin/org/jetbrains/kotlin/script/jsr223/KotlinJsr223JvmDaemonCompileScriptEngine.kt index a891dcf8b1b..2889dd7b274 100644 --- a/libraries/tools/kotlin-script-util/src/main/kotlin/org/jetbrains/kotlin/script/jsr223/KotlinJsr223JvmDaemonCompileScriptEngine.kt +++ b/libraries/tools/kotlin-script-util/src/main/kotlin/org/jetbrains/kotlin/script/jsr223/KotlinJsr223JvmDaemonCompileScriptEngine.kt @@ -30,6 +30,8 @@ import javax.script.ScriptEngineFactory import javax.script.ScriptException import kotlin.reflect.KClass +// TODO: need to manage resources here, i.e. call replCompiler.dispose when engine is collected + class KotlinJsr223JvmDaemonCompileScriptEngine( disposable: Disposable, factory: ScriptEngineFactory, @@ -46,7 +48,6 @@ class KotlinJsr223JvmDaemonCompileScriptEngine( override val replCompiler by lazy { daemon.let { KotlinRemoteReplCompiler( - disposable, it, makeAutodeletingFlagFile("jsr223-repl-session"), CompileService.TargetPlatform.JVM,