Merge pull request #49 from WahdanZ/feature/improvemnts

Fix threading violations, crash bugs, resource leaks, and code cleanup

Author: WahdanZ

CHANGELOG.md

@@ -1,6 +1,26 @@
 <!-- Keep a Changelog guide -> https://keepachangelog.com -->
 
 ## [Unreleased]
+### Fixed
+- **Threading**: `currentBackStack` and `currentApplicationBackStack` now run ADB on a background thread and show popups on EDT; PSI lookups wrapped in `ReadAction.compute`
+- **Crash**: `GetFragmentsCommand` — safe split access with `getOrNull` instead of hardcoded index, avoids `ArrayIndexOutOfBoundsException`
+- **Crash**: `GetApplicationBackStackCommand` — safe array bounds (`getOrNull`) and removed `!!` force-unwrap on `find {}` result
+- **Crash**: `Debugger` — replaced `client!!` with null-safe early return to avoid `NullPointerException` when the debug client is unavailable
+- **Resource leak**: `AdbControllerImp` now implements `Disposable` and removes the `AndroidDebugBridge` device-change listener in `dispose()`
+- **Resource leak**: `BaseAction` — `AdbControllerImp` is disposed immediately after the synchronous device-list read, instead of being incorrectly registered against `Project` as a long-lived disposable parent
+- **Resource leak**: `AdbDrawerViewer` — `AdbControllerImp` is now registered against `toolWindow.disposable` instead of `Project`; the controller's lifetime correctly matches the tool window, not the entire project
+
+### Changed
+- `BaseAction` no longer uses `Disposer.register(project, controller)` — the controller is disposed explicitly after `connectedDevices()` returns, which is safe because the call is synchronous
+
+### Removed
+- Debug `println` statements from `SpockAdbViewer` and `GetApplicationBackStackCommand`
+- Large commented-out dead code blocks in `SpockAdbViewer`, `ConnectDeviceOverIPCommand`, and `CheckBoxDialog`
+- Duplicate empty `setting.addActionListener {}` in `SpockAdbViewer`
+
+### Internal
+- Fixed exception message `"Bazinga!!"` in `GetApplicationPermission` → professional message
+- Renamed `kippAppProcess` → `killAppProcess` (typo fix) in `ProcessDeathCommand`
 
 ## [3.0.0]
 ### Added

src/main/kotlin/spock/AdbDrawerViewer.kt

@@ -4,14 +4,17 @@ import com.intellij.openapi.project.Project
 import com.intellij.openapi.wm.ToolWindow
 import com.intellij.openapi.wm.ToolWindowFactory
 import org.jetbrains.android.sdk.AndroidSdkUtils
-import spock.adb.AdbController
 import spock.adb.AdbControllerImp
 import spock.adb.SpockAdbViewer
 
 class AdbDrawerViewer : ToolWindowFactory {
 
     override fun createToolWindowContent(project: Project, toolWindow: ToolWindow) {
-        val adbController: AdbController = AdbControllerImp(project, AndroidSdkUtils.getDebugBridge(project))
+        val adbController = AdbControllerImp(project, AndroidSdkUtils.getDebugBridge(project))
+        // Use toolWindow.disposable, not project, as the parent — it is disposed when the
+        // tool window is torn down (plugin unload / project close), which is the correct
+        // lifetime for the device-change listener registered inside AdbControllerImp.
+        com.intellij.openapi.util.Disposer.register(toolWindow.disposable, adbController)
         val contentManager = toolWindow.contentManager
 
         with(SpockAdbViewer(project)) {

src/main/kotlin/spock/adb/AdbControllerImp.kt

@@ -20,7 +20,7 @@ import spock.adb.premission.ListItem
 class AdbControllerImp(
     private val project: Project,
     private val debugBridge: AndroidDebugBridge?
-) : AdbController, AndroidDebugBridge.IDeviceChangeListener {
+) : AdbController, AndroidDebugBridge.IDeviceChangeListener, com.intellij.openapi.Disposable {
 
     private var updateDeviceList: ((List<IDevice>) -> Unit)? = null
 
@@ -55,52 +55,72 @@ class AdbControllerImp(
         device: IDevice
 
     ) {
-        val activitiesList = mutableListOf<String>()
-        val activitiesClass: List<BackStackData> = GetBackStackCommand().execute(Any(), project, device)
+        // ADB must run on a background thread — wrap everything in execute {}
+        execute {
+            val activitiesList = mutableListOf<String>()
+            val activitiesClass: List<BackStackData> = GetBackStackCommand().execute(Any(), project, device)
 
-        activitiesClass.forEachIndexed { index, activityData ->
-            activitiesList.add("\t$index-${activityData.appPackage}")
+            activitiesClass.forEachIndexed { index, activityData ->
+                activitiesList.add("\t$index-${activityData.appPackage}")
+                activityData.activitiesList.forEachIndexed { activityIndex, activity ->
+                    activitiesList.add("\t\t\t\t$activityIndex-${activity}")
+                }
+            }
 
-            activityData.activitiesList.forEachIndexed { activityIndex, activity ->
-                activitiesList.add("\t\t\t\t$activityIndex-${activity}")
+            // PSI lookups require a ReadAction when called from a background thread
+            val classes = com.intellij.openapi.application.ReadAction.compute<List<PsiClass?>, RuntimeException> {
+                activitiesList.map { it.trim().substringAfter("-").psiClassByNameFromProjct(project) }
             }
-        }
 
-        showClassPopup(
-            title = "Activities",
-            items = activitiesList,
-            classes = activitiesList.map { it.trim().substringAfter("-").psiClassByNameFromProjct(project) }
-        )
+            // Popup creation and display must happen on the EDT
+            ApplicationManager.getApplication().invokeLater {
+                showClassPopup(title = "Activities", items = activitiesList, classes = classes)
+            }
+        }
     }
 
     override fun currentApplicationBackStack(device: IDevice) {
-        val applicationID = getApplicationID(device)
-        val activitiesList: MutableList<String>
-        val activitiesClass: List<ActivityData> =
-            GetApplicationBackStackCommand().execute(applicationID, project, device)
-        activitiesList = activitiesClass.map { listOf(it.activity) + it.fragment }.flatten().toMutableList()
-        JBPopupFactory.getInstance()
-            .createPopupChooserBuilder(activitiesList)
-            .setTitle("Activities")
-            .setRenderer(javax.swing.ListCellRenderer<String> { _, value, _, _, _ ->
-                var title = value.toString()
-                title = if (!value.toString().contains('.'))
-                    "  |--$title (Fragment)"
-                else
-                    (title.split('.').lastOrNull() ?: "") + "(Activity)"
-                val label = JBLabel(title)
-                label.border = JBUI.Borders.empty(5, 10, 5, 20)
-                label
-            })
-            .setItemChosenCallback { current ->
-                if (current.contains('.'))
-                    current.psiClassByNameFromProjct(project)?.openIn(project)
-                else
-                    current.psiClassByNameFromCache(project)?.openIn(project)
-            }
-            .createPopup()
-            .showCenteredInCurrentWindow(project)
+        // ADB must run on a background thread — wrap everything in execute {}
+        execute {
+            val applicationID = getApplicationID(device)
+            val activitiesClass: List<ActivityData> =
+                GetApplicationBackStackCommand().execute(applicationID, project, device)
+            val activitiesList = activitiesClass.map { listOf(it.activity) + it.fragment }.flatten().toMutableList()
 
+            // Popup creation and display must happen on the EDT
+            ApplicationManager.getApplication().invokeLater {
+                JBPopupFactory.getInstance()
+                    .createPopupChooserBuilder(activitiesList)
+                    .setTitle("Activities")
+                    .setRenderer(javax.swing.ListCellRenderer<String> { _, value, _, _, _ ->
+                        var title = value.toString()
+                        title = if (!value.toString().contains('.'))
+                            "  |--$title (Fragment)"
+                        else
+                            (title.split('.').lastOrNull() ?: "") + "(Activity)"
+                        val label = JBLabel(title)
+                        label.border = JBUI.Borders.empty(5, 10, 5, 20)
+                        label
+                    })
+                    .setItemChosenCallback { current ->
+                        // Item chosen callback runs on EDT; dispatch PSI lookup to background
+                        execute {
+                            val psiClass = com.intellij.openapi.application.ReadAction.compute<PsiClass?, RuntimeException> {
+                                if (current.contains('.'))
+                                    current.psiClassByNameFromProjct(project)
+                                else
+                                    current.psiClassByNameFromCache(project)
+                            }
+                            ApplicationManager.getApplication().invokeLater {
+                                psiClass?.openIn(project)
+                                    ?: showError("class $current Not Found")
+                            }
+                        }
+                    }
+                    .createPopup()
+                    .showCenteredInCurrentWindow(project)
+            }
+        }
     }
 
     override fun currentActivity(
@@ -110,9 +130,12 @@ class AdbControllerImp(
         execute {
             val activity =
                 GetActivityCommand().execute(Any(), project, device) ?: throw Exception("No activities found")
+            // Resolve PSI on background thread inside ReadAction, then open on EDT
+            val psiClass = com.intellij.openapi.application.ReadAction.compute<PsiClass?, RuntimeException> {
+                activity.psiClassByNameFromProjct(project)
+            }
             ApplicationManager.getApplication().invokeLater {
-                activity.psiClassByNameFromProjct(project)?.openIn(project)
-                    ?: showError("class $activity Not Found")
+                psiClass?.openIn(project) ?: showError("class $activity Not Found")
             }
         }
     }
@@ -444,4 +467,9 @@ class AdbControllerImp(
             showSuccess(result)
         }
     }
+
+    override fun dispose() {
+        AndroidDebugBridge.removeDeviceChangeListener(this)
+        updateDeviceList = null
+    }
 }

src/main/kotlin/spock/adb/SpockAdbViewer.kt

@@ -117,7 +117,6 @@ class SpockAdbViewer(
             AppSettingService.getInstance().run {
                 state.let {
                     val dialog = CheckBoxDialog(it.list) { selectedItem ->
-                        println(selectedItem)
                         this.loadState(it.copy(list = it.list.map { item ->
                             if (item.name == selectedItem.name)
                                 item.copy(isSelected = selectedItem.isSelected)
@@ -133,29 +132,10 @@ class SpockAdbViewer(
             }
 
         }
-       adbWifi.isVisible = false
-//        adbWifi.addActionListener {
-//            val ip = Messages.showInputDialog(
-//                "Enter You Android Device IP address",
-//                "Spock Adb- Device connect over Wifi",
-//                null,
-//                "192.168.1.20",
-//                IPAddressInputValidator()
-//            )
-//            ip?.let { adbController.connectDeviceOverIp(ip = ip) }
-
-      //  }
-
-//        refresh.addActionListener {
-//            adbController.refresh()
-//            updateDevicesList()
-//        }
+        adbWifi.isVisible = false
         devicesListComboBox.addItemListener {
             selectedIDevice = devices[devicesListComboBox.selectedIndex]
 
-        }
-        setting.addActionListener {
-
         }
         activitiesBackStackButton.addActionListener {
             selectedIDevice?.let { device ->

src/main/kotlin/spock/adb/actions/BaseAction.kt

@@ -11,21 +11,25 @@ import spock.adb.AdbController
 import spock.adb.AdbControllerImp
 import spock.adb.notification.CommonNotifier.Companion.showNotifier
 
+
 abstract class BaseAction : AnAction() {
     override fun actionPerformed(event: AnActionEvent) = event.project?.run {
         val controller = AdbControllerImp(this, AndroidSdkUtils.getDebugBridge(this))
+        // connectedDevices() is synchronous — it immediately invokes the callback with the
+        // current bridge.devices list, so we can dispose right after to remove the
+        // AndroidDebugBridge device-change listener that init() registered.
         controller.connectedDevices { list ->
             if (list.isNotEmpty())
-                if(list.size > 1)
+                if (list.size > 1)
                     showDeviceList(project = this, devices = list) {
                         performAction(controller, it)
                     }
                 else
                     performAction(controller, list[0])
             else
                 showNotifier(project = this, content = "No Devices", type = NotificationType.ERROR)
-
         }
+        controller.dispose() // remove the device-change listener; async ADB work is unaffected
     } ?: cancelAction()
 
     private fun showDeviceList(project: Project, devices: List<IDevice>, block: (device: IDevice) -> Unit) {

src/main/kotlin/spock/adb/command/ConnectDeviceOverIPCommand.kt

@@ -1,33 +1,9 @@
 package spock.adb.command
 
 import com.intellij.openapi.project.Project
-import java.nio.charset.Charset
-import java.util.concurrent.TimeUnit
-import org.jetbrains.android.sdk.AndroidSdkUtils
 
 class ConnectDeviceOverIPCommand : AdbCommand<String, Any> {
     override fun execute(p: String, project: Project): Any {
-//        val adbPath = AndroidSdkUtils.getAdb(project)?.absolutePath
-//        var process: Process? = null
-//        print("$adbPath connect $p:5555")
-//        try {
-//            process = Runtime.getRuntime().exec("$adbPath connect $p:5555")
-//            if (!process.waitFor(30, TimeUnit.SECONDS)) {
-//                process.run { destroy() }
-//            }
-//            val content = process.errorStream.readBytes().toString(Charset.defaultCharset())
-//            print(content)
-//            process.run {
-//                print(content)
-//                destroy()
-//            }
-//            if (content.isNotEmpty()) throw Exception("enable to connect to $p")
-//            return ""
-//        } catch (e: Exception) {
-//            print(e)
-//            process?.destroy()
-//            throw Exception("enable to connect to $p")
-   //   }
         return ""
     }
 }

src/main/kotlin/spock/adb/command/GetApplicationBackStackCommand.kt

@@ -9,11 +9,9 @@ import java.util.concurrent.TimeUnit
 class GetApplicationBackStackCommand : Command<String, List<ActivityData>> {
 
     companion object {
-        val systemFragments = listOf("ReportFragment", "FragmentManager", "NavHostFragment", "BackStackEntry")
         val currentActiveActivity = Regex("([A-Z])\\w+=true")
         val activityRegex = Regex(" {2}ACTIVITY.*")
         val fragmentRegex = Regex("[a-zA-Z1-9]+\\{[a-z0-9}]")
-        val addedFragmentRegex = Regex("#2: ADD [a-zA-Z1-9]+\\{[a-z0-9}]")
         val removedFragmentRegex = Regex("#1: REMOVE [a-zA-Z1-9]+\\{[a-z0-9}]")
     }
 
@@ -33,9 +31,9 @@ class GetApplicationBackStackCommand : Command<String, List<ActivityData>> {
         val lines = bulkActivitiesData.lines()
         lines.mapIndexed { index, s ->
             if (s.contains(activityRegex)) {
-                val status = currentActiveActivity.find(lines[index + 2])?.value?.split("=")?.firstOrNull()
+                val status = currentActiveActivity.find(lines.getOrNull(index + 2) ?: "")?.value?.split("=")?.firstOrNull()
                     ?: ""
-                val getActivityName = s.split(" ").find { it.contains("/") }!!.replace("/", "")
+                val getActivityName = s.split(" ").find { it.contains("/") }?.replace("/", "") ?: return@mapIndexed
                 tasks.add(
                     ActivityData(
                         activity = getActivityName,
@@ -53,7 +51,6 @@ class GetApplicationBackStackCommand : Command<String, List<ActivityData>> {
                 val removedFragment =
                     removedFragmentRegex.find(s)?.value?.split("{")?.firstOrNull()?.split("REMOVE ")?.lastOrNull()
                 if (removedFragment != null) {
-                    println(removedFragment)
                     tasks[tasks.size - 1] = task.copy(fragment = task.fragment + listOf(removedFragment))
                 }
             }

src/main/kotlin/spock/adb/command/GetApplicationPermission.kt

@@ -38,7 +38,7 @@ class GetApplicationPermission : Command<String, List<ListItem>> {
             } else
                 throw Exception("Application $p not installed")
         } else
-            throw Exception("Bazinga!! Your Device is before Marshmallow. No need to explicitly handle permissions")
+            throw Exception("Device API level is below Marshmallow. Runtime permissions are not supported on this device.")
 
     }
 

src/main/kotlin/spock/adb/command/GetFragmentsCommand.kt

@@ -29,8 +29,9 @@ class GetFragmentsCommand : Command<String, List<FragmentData>> {
     }
 
     private fun getFragmentsUsingOldMethod(bulkTaskDetails: String): List<FragmentData> {
-        return bulkTaskDetails
-            .split("Added Fragments:")[2]
+        val parts = bulkTaskDetails.split("Added Fragments:")
+        val section = parts.getOrNull(2) ?: return mutableListOf()
+        return section
             .lines()
             .map { it.trim() }
             .filter { (it.startsWith("#") && !it.contains("BackStackEntry")) }

src/main/kotlin/spock/adb/command/ProcessDeathCommand.kt

@@ -14,7 +14,7 @@ class ProcessDeathCommand : Command<String, Unit> {
             Thread.sleep(2500L) //If we don't add this delay, the following commands executes without the app
             // being on the background thus not working.
 
-            kippAppProcess(device, p)
+            killAppProcess(device, p)
 
             startApplication(device, p)
         } else {
@@ -28,7 +28,7 @@ class ProcessDeathCommand : Command<String, Unit> {
         }
     }
 
-    private fun kippAppProcess(device: IDevice, p: String) =
+    private fun killAppProcess(device: IDevice, p: String) =
         device.executeShellCommand("am kill $p", ShellOutputReceiver(), 15L, TimeUnit.SECONDS)
 
     private fun startApplication(device: IDevice, p: String) {