Debugger: Fix for coroutine debugger after review

2019-10-01 11:57:02 +03:00
parent 5975251a32
commit 12313fa5aa
9 changed files with 89 additions and 105 deletions
@@ -25,7 +25,7 @@ import com.intellij.openapi.util.registry.Registry
 import com.intellij.psi.search.GlobalSearchScope
 import com.intellij.util.text.DateFormatUtil
 import com.intellij.xdebugger.impl.XDebuggerManagerImpl
-import org.jetbrains.kotlin.idea.debugger.evaluate.createExecutionContext
+import org.jetbrains.kotlin.idea.debugger.evaluate.ExecutionContext

@Suppress("ComponentNotRegistered")
 class CoroutineDumpAction : AnAction(), AnAction.TransparentUpdate {
@@ -39,14 +39,16 @@ class CoroutineDumpAction : AnAction(), AnAction.TransparentUpdate {
            val process = context.debugProcess ?: return
            process.managerThread.schedule(object : SuspendContextCommandImpl(context.suspendContext) {
                override fun contextAction() {
-                    val execContext = context.createExecutionContext() ?: return
+                    val evalContext = context.createEvaluationContext()
+                    val frameProxy = evalContext?.frameProxy ?: return
+                    val execContext = ExecutionContext(evalContext, frameProxy)
                    val states = CoroutinesDebugProbesProxy.dumpCoroutines(execContext)
                    if (states.isLeft) {
                        logger.warn(states.left)
                        XDebuggerManagerImpl.NOTIFICATION_GROUP
                            .createNotification(
                                "Coroutine dump failed. See log",
-                                MessageType.ERROR
+                                MessageType.WARNING
                            ).notify(project)
                        return
                    }
@@ -80,7 +80,7 @@ class CoroutineDumpPanel(project: Project, consoleView: ConsoleView, toolbarActi
            }
        }

-        exporterToTextFile = createToFileExporter(project, dump)
+        exporterToTextFile = MyToFileExporter(project, dump)

        val filterAction = FilterAction().apply {
            registerCustomShortcutSet(
@@ -92,7 +92,7 @@ class CoroutineDumpPanel(project: Project, consoleView: ConsoleView, toolbarActi
            add(filterAction)
            add(CopyToClipboardAction(dump, project))
            add(ActionManager.getInstance().getAction(IdeActions.ACTION_EXPORT_TO_TEXT_FILE))
-            add(MergeStacktracesAction())
+            add(MergeStackTracesAction())
        }
        add(
            ActionManager.getInstance()
@@ -139,7 +139,6 @@ class CoroutineDumpPanel(project: Project, consoleView: ConsoleView, toolbarActi
        val states = if (UISettings.instance.state.mergeEqualStackTraces) mergedDump else dump
        for (state in states) {
            if (StringUtil.containsIgnoreCase(state.stringStackTrace, text) || StringUtil.containsIgnoreCase(state.name, text)) {
-
                model.addElement(state)
                if (selection === state) {
                    selectedIndex = index
@@ -211,9 +210,11 @@ class CoroutineDumpPanel(project: Project, consoleView: ConsoleView, toolbarActi
        }
    }

-    private inner class FilterAction :
-        ToggleAction("Filter", "Show only threads containing a specific string", AllIcons.General.Filter),
-        DumbAware {
+    private inner class FilterAction : ToggleAction(
+        "Filter",
+        "Show only coroutines containing a specific string",
+        AllIcons.General.Filter
+    ), DumbAware {

        override fun isSelected(e: AnActionEvent): Boolean {
            return filterPanel.isVisible
@@ -229,14 +230,11 @@ class CoroutineDumpPanel(project: Project, consoleView: ConsoleView, toolbarActi
        }
    }

-    @Suppress("DEPRECATION")
-    private inner class MergeStacktracesAction :
-        ToggleAction(
-            "Merge Identical Stacktraces",
-            "Group coroutines with identical stacktraces",
-            AllIcons.General.CollapseAll
-        ),
-        DumbAware {
+    private inner class MergeStackTracesAction : ToggleAction(
+        "Merge Identical Stacktraces",
+        "Group coroutines with identical stacktraces",
+        AllIcons.Actions.Collapseall
+    ), DumbAware {

        override fun isSelected(e: AnActionEvent): Boolean {
            return UISettings.instance.state.mergeEqualStackTraces
@@ -268,32 +266,19 @@ class CoroutineDumpPanel(project: Project, consoleView: ConsoleView, toolbarActi
        private val group = NotificationGroup.toolWindowGroup("Analyze coroutine dump", ToolWindowId.RUN, false)
    }

-    private fun createToFileExporter(project: Project, states: List<CoroutineState>): ExporterToTextFile {
-        return MyToFileExporter(project, states)
-    }
+    private class MyToFileExporter(
+        private val myProject: Project,
+        private val states: List<CoroutineState>
+    ) : ExporterToTextFile {

-    private class MyToFileExporter(private val myProject: Project, private val states: List<CoroutineState>) :
-        ExporterToTextFile {
-
-        override fun getReportText(): String {
-            val sb = StringBuilder()
-            for (state in states) {
-                sb.append(state.stringStackTrace).append("\n\n")
-            }
-            return sb.toString()
+        override fun getReportText() = buildString {
+            for (state in states)
+                append(state.stringStackTrace).append("\n\n")
        }

-        @Suppress("DEPRECATION")
-        override fun getDefaultFilePath(): String {
-            val baseDir = myProject.baseDir
-            return if (baseDir != null) {
-                baseDir.presentableUrl + File.separator + defaultReportFileName
-            } else ""
-        }
+        override fun getDefaultFilePath() = (myProject.basePath ?: "") + File.separator + defaultReportFileName

-        override fun canExport(): Boolean {
-            return states.isNotEmpty()
-        }
+        override fun canExport() = states.isNotEmpty()

        private val defaultReportFileName = "coroutines_report.txt"
    }
@@ -61,13 +61,13 @@ class CoroutineState(
            val trace = context.invokeMethod(continuation, getTrace, emptyList()) as? ObjectReference
            if (trace != null)
                (context.invokeMethod(trace, getClassName, emptyList()) as StringReference).value()
-            else ""
+            else null
        }
        val lineNumber = {
            val trace = context.invokeMethod(continuation, getTrace, emptyList()) as? ObjectReference
            if (trace != null)
                (context.invokeMethod(trace, getLineNumber, emptyList()) as IntegerValue).value()
-            else -239 // invalid line number (but well-educated)
+            else null
        }

        while (continuation.type().isSubtype(baseType)
@@ -40,7 +40,7 @@ class CoroutinesDebugConfigurationExtension : RunConfigurationExtension() {
    private var Project.listenerCreated: Boolean? by UserDataProperty(Key.create("COROUTINES_DEBUG_TAB_CREATE_LISTENER"))

    override fun isApplicableFor(configuration: RunConfigurationBase<*>): Boolean {
-        return Registry.`is`("kotlin.debugger" + ".coroutines")
+        return Registry.`is`("kotlin.debugger.coroutines")
    }

    override fun <T : RunConfigurationBase<*>?> updateJavaParameters(
@@ -48,7 +48,7 @@ class CoroutinesDebugConfigurationExtension : RunConfigurationExtension() {
        params: JavaParameters?,
        runnerSettings: RunnerSettings?
    ) {
-        if (!Registry.`is`("kotlin.debugger" + ".coroutines")) return
+        if (!Registry.`is`("kotlin.debugger.coroutines")) return
        if (runnerSettings is DebuggingRunnerData && params != null
            && params.classPath != null
            && params.classPath.pathList.isNotEmpty()
@@ -78,8 +78,9 @@ class CoroutinesDebugConfigurationExtension : RunConfigurationExtension() {
            override fun processStarted(debugProcess: XDebugProcess) {
                DebuggerInvocationUtil.swingInvokeLater(project) {
                    val session = DebuggerManagerEx.getInstanceEx(project).context.debuggerSession
-                    if (session != null)
-                        registerCoroutinesPanel(session.xDebugSession?.ui ?: return@swingInvokeLater, session)
+                    val ui = session?.xDebugSession?.ui
+                    if (ui != null)
+                        registerCoroutinesPanel(ui, session)
                }
            }

@@ -16,6 +16,7 @@ object CoroutinesDebugProbesProxy {
    private const val DEBUG_PACKAGE = "kotlinx.coroutines.debug"
    private var DebugProcess.references by UserDataProperty(Key.create<ProcessReferences>("COROUTINES_DEBUG_REFERENCES"))

+    @Synchronized
    @Suppress("unused")
    fun install(context: ExecutionContext) {
        val debugProbes = context.findClass("$DEBUG_PACKAGE.DebugProbes") as ClassType
@@ -24,6 +25,7 @@ object CoroutinesDebugProbesProxy {
        context.invokeMethod(instance, install, emptyList())
    }

+    @Synchronized
    @Suppress("unused")
    fun uninstall(context: ExecutionContext) {
        val debugProbes = context.findClass("$DEBUG_PACKAGE.DebugProbes") as ClassType
@@ -36,6 +38,7 @@ object CoroutinesDebugProbesProxy {
     * Invokes DebugProbes from debugged process's classpath and returns states of coroutines
     * Should be invoked on debugger manager thread
     */
+    @Synchronized
    fun dumpCoroutines(context: ExecutionContext): Either<Throwable, List<CoroutineState>> {
        try {
            if (context.debugProcess.references == null) {
@@ -50,6 +53,7 @@ object CoroutinesDebugProbesProxy {

            return Either.right(List(size) {
                val index = context.vm.mirrorOf(it)
+                // `List<CoroutineInfo>.get(index)`
                val elem = context.invokeMethod(infoList, refs.getElement, listOf(index)) as ObjectReference
                val name = getName(context, elem, refs)
                val state = getState(context, elem, refs)
@@ -73,7 +77,11 @@ object CoroutinesDebugProbesProxy {
        refs: ProcessReferences
    ): String {
        // equals to `coroutineInfo.context.get(CoroutineName).name`
-        val coroutineContextInst = context.invokeMethod(info, refs.getContext, emptyList()) as ObjectReference
+        val coroutineContextInst = context.invokeMethod(
+            info,
+            refs.getContext,
+            emptyList()
+        ) as? ObjectReference ?: throw IllegalArgumentException("Coroutine context must not be null")
        val coroutineName = context.invokeMethod(
            coroutineContextInst,
            refs.getContextElement, listOf(refs.nameKey)
@@ -81,7 +89,8 @@ object CoroutinesDebugProbesProxy {
        // If the coroutine doesn't have a given name, CoroutineContext.get(CoroutineName) returns null
        val name = if (coroutineName != null) (context.invokeMethod(
            coroutineName,
-            refs.getName, emptyList()
+            refs.getName,
+            emptyList()
        ) as StringReferenceImpl).value() else "coroutine"
        val id = (info.getValue(refs.idField) as LongValue).value()
        return "$name#$id"
@@ -92,7 +101,7 @@ object CoroutinesDebugProbesProxy {
        info: ObjectReference, // CoroutineInfo instance
        refs: ProcessReferences
    ): String {
-        //  equals to stringState = coroutineInfo.state.toString()
+        // equals to `stringState = coroutineInfo.state.toString()`
        val state = context.invokeMethod(info, refs.getState, emptyList()) as ObjectReference
        return (context.invokeMethod(state, refs.toString, emptyList()) as StringReferenceImpl).value()
    }
@@ -100,7 +109,7 @@ object CoroutinesDebugProbesProxy {
    private fun getLastObservedThread(
        info: ObjectReference, // CoroutineInfo instance
        threadRef: Field // reference to lastObservedThread
-    ): ThreadReference? = info.getValue(threadRef) as ThreadReference?
+    ): ThreadReference? = info.getValue(threadRef) as? ThreadReference

    /**
     * Returns list of stackTraceElements for the given CoroutineInfo's [ObjectReference]
@@ -124,14 +133,12 @@ object CoroutinesDebugProbesProxy {
                listOf(context.vm.virtualMachine.mirrorOf(it))
            ) as ObjectReference
            val clazz = (frame.getValue(refs.className) as StringReference).value()
-
-//            if (clazz.contains("$DEBUG_PACKAGE.DebugProbes")) break // cut off debug intrinsic stacktrace
            list.add(
                0, // add in the beginning
                StackTraceElement(
                    clazz,
-                    (frame.getValue(refs.methodName) as StringReference).value(),
-                    (frame.getValue(refs.fileName) as StringReference?)?.value(),
+                    (frame.getValue(refs.methodName) as? StringReference)?.value(),
+                    (frame.getValue(refs.fileName) as? StringReference)?.value(),
                    (frame.getValue(refs.line) as IntegerValue).value()
                )
            )
@@ -165,7 +172,7 @@ object CoroutinesDebugProbesProxy {
        val getName: Method = coroutineName.methodsByName("getName").single()
        val nameKey = coroutineName.getValue(coroutineName.fieldByName("Key")) as ObjectReference
        val toString: Method = (context.findClass("java.lang.Object") as ClassType)
-            .methodsByName("toString").single()
+            .concreteMethodByName("toString", "()Ljava/lang/String;")

        val threadRef: Field = info.fieldByName("lastObservedThread")
        val continuation: Field = info.fieldByName("lastObservedFrame")
@@ -41,7 +41,6 @@ import com.sun.jdi.ClassType
 import javaslang.control.Either
 import org.jetbrains.kotlin.idea.debugger.KotlinCoroutinesAsyncStackTraceProvider
 import org.jetbrains.kotlin.idea.debugger.evaluate.ExecutionContext
-import org.jetbrains.kotlin.idea.debugger.evaluate.createExecutionContext
 import java.awt.event.MouseEvent
 import java.lang.ref.WeakReference
 import javax.swing.event.TreeModelEvent
@@ -52,7 +51,7 @@ import javax.swing.event.TreeModelListener
 */
 class CoroutinesDebuggerTree(project: Project) : DebuggerTree(project) {
    private val logger = Logger.getInstance(this::class.java)
-    private var lastSuspendContextDump: Pair<WeakReference<SuspendContextImpl>, Either<Throwable, List<CoroutineState>>>? = null
+    private var lastSuspendContextCache: Cache? = null

    override fun createNodeManager(project: Project): NodeManagerImpl {
        return object : NodeManagerImpl(project, this) {
@@ -227,11 +226,13 @@ class CoroutinesDebuggerTree(project: Project) : DebuggerTree(project) {
        pos: XSourcePosition
    ): Pair<XExecutionStack, SyntheticStackFrame>? {
        val context = DebuggerManagerEx.getInstanceEx(project).context
-        val proxy = context.suspendContext?.thread ?: return null
-        val executionStack =
-            JavaExecutionStack(proxy, context.debugProcess!!, false)
+        val suspendContext = context.suspendContext ?: return null
+        val proxy = suspendContext.thread ?: return null
+        val executionStack = JavaExecutionStack(proxy, suspendContext.debugProcess, false)
        executionStack.initTopFrame()
-        val execContext = context.createExecutionContext() ?: return null
+        val evalContext = context.createEvaluationContext()
+        val frameProxy = evalContext?.frameProxy ?: return null
+        val execContext = ExecutionContext(evalContext, frameProxy)
        val continuation = descriptor.continuation // guaranteed that it is a BaseContinuationImpl
        val aMethod = (continuation.type() as ClassType).concreteMethodByName(
            "getStackTraceElement",
@@ -255,6 +256,7 @@ class CoroutinesDebuggerTree(project: Project) : DebuggerTree(project) {
        descriptor: NodeDescriptorImpl,
        evalContext: EvaluationContextImpl
    ) {
+        val creationStackTraceSeparator = "\b\b\b" // the "\b\b\b" is used for creation stacktrace separator in kotlinx.coroutines
        if (descriptor !is CoroutineDescriptorImpl) {
            if (descriptor is CreationFramesDescriptor) {
                val threadProxy = debuggerContext.suspendContext?.thread ?: return
@@ -298,7 +300,7 @@ class CoroutinesDebuggerTree(project: Project) : DebuggerTree(project) {
                val proxy = threadProxy.forceFrames().first()
                // the thread is paused on breakpoint - it has at least one frame
                for (it in descriptor.state.stackTrace) {
-                    if (it.className.startsWith("\b\b\b")) break
+                    if (it.className.startsWith(creationStackTraceSeparator)) break
                    children.add(createCoroutineFrameDescriptor(descriptor, evalContext, it, proxy))
                }
            }
@@ -306,7 +308,7 @@ class CoroutinesDebuggerTree(project: Project) : DebuggerTree(project) {
            }
        }
        val trace = descriptor.state.stackTrace
-        val index = trace.indexOfFirst { it.className.startsWith("\b\b\b") }
+        val index = trace.indexOfFirst { it.className.startsWith(creationStackTraceSeparator) }
        children.add(myNodeManager.createNode(CreationFramesDescriptor(trace.subList(index + 1, trace.size)), evalContext))
    }

@@ -371,7 +373,7 @@ class CoroutinesDebuggerTree(project: Project) : DebuggerTree(project) {

    override fun isExpandable(node: DebuggerTreeNodeImpl): Boolean {
        val descriptor = node.descriptor
-        return if (descriptor is StackFrameDescriptor) return false else descriptor.isExpandable
+        return if (descriptor is StackFrameDescriptor) false else descriptor.isExpandable
    }

    override fun build(context: DebuggerContextImpl) {
@@ -402,11 +404,11 @@ class CoroutinesDebuggerTree(project: Project) : DebuggerTree(project) {
            }
            val evaluationContext = EvaluationContextImpl(suspendContext, suspendContext.frameProxy)
            val executionContext = ExecutionContext(evaluationContext, suspendContext.frameProxy ?: return)
-            val cache = lastSuspendContextDump
+            val cache = lastSuspendContextCache
            val states = if (cache != null && cache.first.get() === suspendContext) {
                cache.second
            } else CoroutinesDebugProbesProxy.dumpCoroutines(executionContext).apply {
-                lastSuspendContextDump = WeakReference(suspendContext) to this
+                lastSuspendContextCache = WeakReference(suspendContext) to this
            }
            // if suspend context hasn't changed - use last dump, else compute new
            if (states.isLeft) {
@@ -441,4 +443,6 @@ class CoroutinesDebuggerTree(project: Project) : DebuggerTree(project) {

    }

-}
+}
+
+private typealias Cache = Pair<WeakReference<SuspendContextImpl>, Either<Throwable, List<CoroutineState>>>
@@ -120,6 +120,7 @@ class CoroutinesPanel(project: Project, stateManager: DebuggerStateManager) : De
        super.dispose()
    }

+    // copied from com.intellij.debugger.ui.impl.ThreadsPanel.updateNodeLabels
    private fun updateNodeLabels(from: DebuggerTreeNodeImpl) {
        val children = from.children()
        try {
@@ -25,6 +25,7 @@ import com.intellij.openapi.util.IconLoader
 import com.sun.jdi.ClassType
 import com.sun.jdi.ObjectReference
 import org.jetbrains.kotlin.idea.IconExtensionChooser
+import javaslang.control.Either
 import org.jetbrains.kotlin.idea.debugger.evaluate.ExecutionContext
 import javax.swing.Icon

@@ -37,29 +38,16 @@ class CoroutineData(private val state: CoroutineState) : DescriptorData<Coroutin
        return CoroutineDescriptorImpl(state)
    }

-    override fun equals(other: Any?): Boolean {
-        return if (other !is CoroutineData) {
-            false
-        } else state.name == other.state.name
-    }
+    override fun equals(other: Any?) = if (other !is CoroutineData) false else state.name == other.state.name

-    override fun hashCode(): Int {
-        return state.name.hashCode()
-    }
+    override fun hashCode() = state.name.hashCode()

-    override fun getDisplayKey(): DisplayKey<CoroutineDescriptorImpl> {
-        return SimpleDisplayKey(state.name)
-    }
+    override fun getDisplayKey(): DisplayKey<CoroutineDescriptorImpl> = SimpleDisplayKey(state.name)
 }

 class CoroutineDescriptorImpl(val state: CoroutineState) : NodeDescriptorImpl() {
    var suspendContext: SuspendContextImpl? = null
-    val icon: Icon
-        get() = when {
-            state.isSuspended -> AllIcons.Debugger.ThreadSuspended
-            state.state == CoroutineState.State.CREATED -> AllIcons.Debugger.ThreadStates.Idle
-            else -> AllIcons.Debugger.ThreadRunning
-        }
+    lateinit var icon: Icon

    override fun getName(): String? {
        return state.name
@@ -76,30 +64,35 @@ class CoroutineDescriptorImpl(val state: CoroutineState) : NodeDescriptorImpl()
        return state.state != CoroutineState.State.CREATED
    }

-    override fun setContext(context: EvaluationContextImpl?) {
+    private fun calcIcon() = when {
+        state.isSuspended -> AllIcons.Debugger.ThreadSuspended
+        state.state == CoroutineState.State.CREATED -> AllIcons.Debugger.ThreadStates.Idle
+        else -> AllIcons.Debugger.ThreadRunning
+    }

+    override fun setContext(context: EvaluationContextImpl?) {
+        icon = calcIcon()
    }
 }

 class CoroutineStackFrameData private constructor(val state: CoroutineState, private val proxy: StackFrameProxyImpl) :
    DescriptorData<NodeDescriptorImpl>() {
-    private var frame: StackTraceElement? = null
-    private var frameItem: StackFrameItem? = null
+    private lateinit var frame: Either<StackTraceElement, StackFrameItem>

    constructor(state: CoroutineState, frame: StackTraceElement, proxy: StackFrameProxyImpl) : this(state, proxy) {
-        this.frame = frame
+        this.frame = Either.left(frame)
    }

    constructor(state: CoroutineState, frameItem: StackFrameItem, proxy: StackFrameProxyImpl) : this(state, proxy) {
-        this.frameItem = frameItem
+        this.frame = Either.right(frameItem)
    }

-    override fun hashCode() = frame?.hashCode() ?: frameItem.hashCode()
+    override fun hashCode(): Int {
+        return if (frame.isLeft) frame.left.hashCode() else frame.get().hashCode()
+    }

    override fun equals(other: Any?): Boolean {
-        return if (other is CoroutineStackFrameData) {
-            other.frame == frame && other.frameItem == frameItem
-        } else false
+        return other is CoroutineStackFrameData && frame == other.frame
    }

    /**
@@ -107,12 +100,10 @@ class CoroutineStackFrameData private constructor(val state: CoroutineState, pri
     * or [AsyncStackFrameDescriptor] according to current frame
     */
    override fun createDescriptorImpl(project: Project): NodeDescriptorImpl {
-        val frame = frame ?: return AsyncStackFrameDescriptor(
-            state,
-            frameItem!!,
-            proxy
-        )
+        val isLeft = frame.isLeft
+        if (!isLeft) return AsyncStackFrameDescriptor(state, frame.get(), proxy)
        // check whether last fun is suspend fun
+        val frame = frame.left
        val suspendContext =
            DebuggerManagerEx.getInstanceEx(project).context.suspendContext ?: return EmptyStackFrameDescriptor(
                frame,
@@ -212,9 +203,7 @@ class EmptyStackFrameDescriptor(val frame: StackTraceElement, proxy: StackFrameP
 }

 class CreationFramesDescriptor(val frames: List<StackTraceElement>) : NodeDescriptorImpl() {
-    override fun calcRepresentation(context: EvaluationContextImpl?, labelListener: DescriptorLabelListener?): String {
-        return "Coroutine creation stack trace"
-    }
+    override fun calcRepresentation(context: EvaluationContextImpl?, labelListener: DescriptorLabelListener?) = name

    override fun setContext(context: EvaluationContextImpl?) {}
    override fun getName() = "Coroutine creation stack trace"
@@ -100,9 +100,4 @@ class ExecutionContext(val evaluationContext: EvaluationContextImpl, val framePr
        @Suppress("DEPRECATION")
        DebuggerUtilsEx.keep(reference, evaluationContext)
    }
-}
-
-fun DebuggerContextImpl.createExecutionContext(): ExecutionContext? {
-    val context = createEvaluationContext()
-    return ExecutionContext(context ?: return null, frameProxy ?: return null)
 }