From e38a41d2f27b1f7f8a763d367bf0ce98f725db8a Mon Sep 17 00:00:00 2001 From: Yan Zhulanow Date: Mon, 22 Jan 2018 04:17:15 +0300 Subject: [PATCH] Evaluate: Fix compiling evaluator issues led to improper expression caching 1. Do not save a ClassLoader reference inside the context, as we don't use it anyway after evaluation. This is to avoid custom ClassLoader nesting. 2. Do not use 'findClass()' as it caches the loaded classes and always returns the first evaluated class if it's in cache. --- .../evaluate/KotlinEvaluationBuilder.kt | 34 +++++++------------ .../AndroidOClassLoadingAdapter.kt | 4 +-- .../classLoading/ClassLoaderHandler.kt | 24 ------------- .../classLoading/ClassLoadingAdapter.kt | 5 +-- .../OrdinaryClassLoadingAdapter.kt | 4 +-- .../debugger/evaluate/compilingEvaluator.kt | 20 +++++------ 6 files changed, 29 insertions(+), 62 deletions(-) delete mode 100644 idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/classLoading/ClassLoaderHandler.kt diff --git a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinEvaluationBuilder.kt b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinEvaluationBuilder.kt index 60b4701395c..ad65c55efed 100644 --- a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinEvaluationBuilder.kt +++ b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinEvaluationBuilder.kt @@ -64,7 +64,6 @@ import org.jetbrains.kotlin.idea.debugger.DebuggerUtils import org.jetbrains.kotlin.idea.debugger.evaluate.KotlinDebuggerCaches.CompiledDataDescriptor import org.jetbrains.kotlin.idea.debugger.evaluate.KotlinDebuggerCaches.ParametersDescriptor import org.jetbrains.kotlin.idea.debugger.evaluate.classLoading.ClassToLoad -import org.jetbrains.kotlin.idea.debugger.evaluate.compilingEvaluator.loadClasses import org.jetbrains.kotlin.idea.debugger.evaluate.compilingEvaluator.loadClassesSafely import org.jetbrains.kotlin.idea.refactoring.introduce.extractionEngine.ExtractionResult import org.jetbrains.kotlin.idea.runInReadActionWithWriteActionPriorityWithPCE @@ -142,14 +141,10 @@ class KotlinEvaluator(val codeFragment: KtCodeFragment, val sourcePosition: Sour extractAndCompile(fragment, position, context) } - val classLoaderHandler = loadClassesSafely(context, compiledData.classes) + val classLoaderRef = loadClassesSafely(context, compiledData.classes) - val result = if (classLoaderHandler != null) { - try { - evaluateWithCompilation(context, compiledData) ?: runEval4j(context, compiledData) - } finally { - classLoaderHandler.dispose() - } + val result = if (classLoaderRef != null) { + evaluateWithCompilation(context, compiledData, classLoaderRef) ?: runEval4j(context, compiledData) } else { runEval4j(context, compiledData) @@ -157,14 +152,7 @@ class KotlinEvaluator(val codeFragment: KtCodeFragment, val sourcePosition: Sour // If bytecode was taken from cache and exception was thrown - recompile bytecode and run eval4j again if (isCompiledDataFromCache && result is ExceptionThrown && result.kind == ExceptionThrown.ExceptionKind.BROKEN_CODE) { - // We need only lambda classes here cause we using only eval4j evaluation method - val classLoaderHandler = loadClasses(context, compiledData.classes.filter { !it.isMainClass() }) - - try { - return runEval4j(context, extractAndCompile(codeFragment, sourcePosition, context)).toJdiValue(context) - } finally { - classLoaderHandler?.dispose() - } + return runEval4j(context, extractAndCompile(codeFragment, sourcePosition, context)).toJdiValue(context) } return if (result is InterpreterResult) { @@ -262,28 +250,30 @@ class KotlinEvaluator(val codeFragment: KtCodeFragment, val sourcePosition: Sour get() = classes.firstOrNull { it.isMainClass() } ?: error( "Can't find main class for " + sourcePosition.elementAt.getParentOfType(strict = false)) - private fun evaluateWithCompilation(context: EvaluationContextImpl, compiledData: CompiledDataDescriptor): Any? { + private fun evaluateWithCompilation( + context: EvaluationContextImpl, + compiledData: CompiledDataDescriptor, + classLoader: ClassLoaderReference + ): Any? { val vm = context.debugProcess.virtualMachineProxy.virtualMachine - val classLoader = context.classLoader ?: return null val mainClassBytecode = compiledData.mainClass.bytes try { val mainClassAsmNode = ClassNode().apply { ClassReader(mainClassBytecode).accept(this, ClassReader.SKIP_CODE) } - val mainClassJdiName = mainClassAsmNode.name.replace('/', '.') assert(mainClassAsmNode.methods.size == 1) val methodToInvoke = mainClassAsmNode.methods[0] assert(methodToInvoke.parameters == null || methodToInvoke.parameters.isEmpty()) - val mainClass = context.debugProcess.findClass(context, mainClassJdiName, classLoader) as ClassType - val thread = context.suspendContext.thread?.threadReference!! val invokePolicy = context.suspendContext.getInvokePolicy() val eval = JDIEval(vm, classLoader, thread, invokePolicy) + val mainClassValue = (eval.loadClass(Type.getObjectType(mainClassAsmNode.name), classLoader) as? ObjectValue) + val mainClass = (mainClassValue?.value as? ClassObjectReference)?.reflectedType() as? ClassType ?: return null + return vm.executeWithBreakpointsDisabled { // Prepare the main class - eval.loadClass(Type.getObjectType(mainClassAsmNode.name), classLoader) val argumentTypes = Type.getArgumentTypes(methodToInvoke.desc) val args = context.getArgumentsForEval4j(compiledData.parameters, argumentTypes) diff --git a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/classLoading/AndroidOClassLoadingAdapter.kt b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/classLoading/AndroidOClassLoadingAdapter.kt index a056b70806f..6f047233c6c 100644 --- a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/classLoading/AndroidOClassLoadingAdapter.kt +++ b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/classLoading/AndroidOClassLoadingAdapter.kt @@ -37,7 +37,7 @@ class AndroidOClassLoadingAdapter : AbstractAndroidClassLoadingAdapter() { } } - override fun loadClasses(context: EvaluationContextImpl, classes: Collection): ClassLoaderHandler { + override fun loadClasses(context: EvaluationContextImpl, classes: Collection): ClassLoaderReference { val process = context.debugProcess val inMemoryClassLoaderClass = resolveClassLoaderClass(context) ?: error("InMemoryDexClassLoader class not found") val constructorMethod = inMemoryClassLoaderClass.concreteMethodByName( @@ -52,6 +52,6 @@ class AndroidOClassLoadingAdapter : AbstractAndroidClassLoadingAdapter() { DebuggerUtilsEx.keep(newClassLoader, context) - return ClassLoaderHandler(newClassLoader as ClassLoaderReference) + return newClassLoader as ClassLoaderReference } } \ No newline at end of file diff --git a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/classLoading/ClassLoaderHandler.kt b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/classLoading/ClassLoaderHandler.kt deleted file mode 100644 index 94f4eb18de4..00000000000 --- a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/classLoading/ClassLoaderHandler.kt +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright 2010-2017 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.jetbrains.kotlin.idea.debugger.evaluate.classLoading - -import com.intellij.openapi.Disposable -import com.sun.jdi.ClassLoaderReference - -open class ClassLoaderHandler(val reference: ClassLoaderReference?) : Disposable { - override fun dispose() {} -} \ No newline at end of file diff --git a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/classLoading/ClassLoadingAdapter.kt b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/classLoading/ClassLoadingAdapter.kt index 21867f39d0d..075e9f09e15 100644 --- a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/classLoading/ClassLoadingAdapter.kt +++ b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/classLoading/ClassLoadingAdapter.kt @@ -21,6 +21,7 @@ import com.intellij.debugger.engine.evaluation.EvaluationContextImpl import com.intellij.debugger.impl.DebuggerUtilsEx import com.sun.jdi.ArrayReference import com.sun.jdi.ArrayType +import com.sun.jdi.ClassLoaderReference import com.sun.jdi.Value import org.jetbrains.org.objectweb.asm.ClassReader import org.jetbrains.org.objectweb.asm.Label @@ -34,7 +35,7 @@ interface ClassLoadingAdapter { AndroidOClassLoadingAdapter(), OrdinaryClassLoadingAdapter()) - fun loadClasses(context: EvaluationContextImpl, classes: Collection): ClassLoaderHandler? { + fun loadClasses(context: EvaluationContextImpl, classes: Collection): ClassLoaderReference? { val hasAdditionalClasses = classes.size > 1 val hasLoops = classes.isNotEmpty() && doesContainLoops(classes.first { it.isMainClass() }.bytes) @@ -76,7 +77,7 @@ interface ClassLoadingAdapter { fun isApplicable(context: EvaluationContextImpl, hasAdditionalClasses: Boolean, hasLoops: Boolean): Boolean - fun loadClasses(context: EvaluationContextImpl, classes: Collection): ClassLoaderHandler + fun loadClasses(context: EvaluationContextImpl, classes: Collection): ClassLoaderReference fun mirrorOfByteArray(bytes: ByteArray, context: EvaluationContextImpl, process: DebugProcessImpl): ArrayReference { val arrayClass = process.findClass(context, "byte[]", context.classLoader) as ArrayType diff --git a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/classLoading/OrdinaryClassLoadingAdapter.kt b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/classLoading/OrdinaryClassLoadingAdapter.kt index f03adb7fccf..b7163d99137 100644 --- a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/classLoading/OrdinaryClassLoadingAdapter.kt +++ b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/classLoading/OrdinaryClassLoadingAdapter.kt @@ -41,7 +41,7 @@ class OrdinaryClassLoadingAdapter : ClassLoadingAdapter { return (hasAdditionalClasses || hasLoops) && context.classLoader != null && !context.debugProcess.isDexDebug() } - override fun loadClasses(context: EvaluationContextImpl, classes: Collection): ClassLoaderHandler { + override fun loadClasses(context: EvaluationContextImpl, classes: Collection): ClassLoaderReference { val process = context.debugProcess val classLoader = try { @@ -68,7 +68,7 @@ class OrdinaryClassLoadingAdapter : ClassLoadingAdapter { throw EvaluateException("Error during classes definition " + e, e) } - return ClassLoaderHandler(classLoader) + return classLoader } private fun defineClasses( diff --git a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/compilingEvaluator.kt b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/compilingEvaluator.kt index ce78c27406a..9c96bbe1ed9 100644 --- a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/compilingEvaluator.kt +++ b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/compilingEvaluator.kt @@ -17,24 +17,24 @@ package org.jetbrains.kotlin.idea.debugger.evaluate.compilingEvaluator import com.intellij.debugger.engine.evaluation.EvaluationContextImpl +import com.sun.jdi.ClassLoaderReference import org.jetbrains.kotlin.idea.debugger.evaluate.LOG -import org.jetbrains.kotlin.idea.debugger.evaluate.classLoading.ClassLoaderHandler import org.jetbrains.kotlin.idea.debugger.evaluate.classLoading.ClassLoadingAdapter import org.jetbrains.kotlin.idea.debugger.evaluate.classLoading.ClassToLoad -fun loadClassesSafely(evaluationContext: EvaluationContextImpl, classes: Collection): ClassLoaderHandler? { - try { - return loadClasses(evaluationContext, classes) +fun loadClassesSafely(evaluationContext: EvaluationContextImpl, classes: Collection): ClassLoaderReference? { + return try { + loadClasses(evaluationContext, classes) } catch (e: Throwable) { LOG.debug("Failed to evaluate expression", e) - return null + null } } -fun loadClasses(evaluationContext: EvaluationContextImpl, classes: Collection): ClassLoaderHandler? { - if (classes.isEmpty()) return ClassLoaderHandler(evaluationContext.classLoader) - - return ClassLoadingAdapter.loadClasses(evaluationContext, classes)?.apply { - evaluationContext.classLoader = this.reference +fun loadClasses(evaluationContext: EvaluationContextImpl, classes: Collection): ClassLoaderReference? { + if (classes.isEmpty()) { + return null } + + return ClassLoadingAdapter.loadClasses(evaluationContext, classes) } \ No newline at end of file