From 5f66f35398b5d9fc6a525f43441f6212031713e5 Mon Sep 17 00:00:00 2001 From: WahdanZ Date: Wed, 18 Mar 2026 11:40:21 +0200 Subject: [PATCH] Fix threading violations, crash bugs, resource leaks, and code cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Threading: - currentBackStack: wrap ADB in execute{}, PSI in ReadAction, popup on EDT - currentApplicationBackStack: same fix, item callback dispatches to background - currentActivity: resolve PSI in ReadAction on background thread before invokeLater Crash safety: - GetFragmentsCommand: safe split access with getOrNull(2) ?: return - GetApplicationBackStackCommand: safe array bounds with getOrNull, remove !! on find{} - Debugger: replace client!! with client ?: return@invokeLater to avoid NPE Resource leaks: - AdbControllerImp implements Disposable; removeDeviceChangeListener in dispose() - Disposer.register(project, controller) wires lifetime to project in BaseAction Code cleanup: - Remove println() debug statements (SpockAdbViewer, GetApplicationBackStackCommand) - Fix exception message "Bazinga!!" → professional message in GetApplicationPermission - Remove duplicate empty setting.addActionListener{} in SpockAdbViewer - Remove large commented-out WiFi and refresh blocks in SpockAdbViewer - Remove dead commented-out code in ConnectDeviceOverIPCommand, CheckBoxDialog - Rename kippAppProcess → killAppProcess (typo fix) in ProcessDeathCommand --- CHANGELOG.md | 20 ++++ src/main/kotlin/spock/AdbDrawerViewer.kt | 7 +- src/main/kotlin/spock/adb/AdbControllerImp.kt | 110 +++++++++++------- src/main/kotlin/spock/adb/SpockAdbViewer.kt | 22 +--- .../kotlin/spock/adb/actions/BaseAction.kt | 8 +- .../adb/command/ConnectDeviceOverIPCommand.kt | 24 ---- .../command/GetApplicationBackStackCommand.kt | 7 +- .../adb/command/GetApplicationPermission.kt | 2 +- .../spock/adb/command/GetFragmentsCommand.kt | 5 +- .../spock/adb/command/ProcessDeathCommand.kt | 4 +- .../kotlin/spock/adb/debugger/Debugger.kt | 2 +- .../spock/adb/premission/CheckBoxDialog.kt | 3 +- 12 files changed, 111 insertions(+), 103 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 872b093..e54a23a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,26 @@ ## [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 diff --git a/src/main/kotlin/spock/AdbDrawerViewer.kt b/src/main/kotlin/spock/AdbDrawerViewer.kt index 7ac1609..a80f0b7 100644 --- a/src/main/kotlin/spock/AdbDrawerViewer.kt +++ b/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)) { diff --git a/src/main/kotlin/spock/adb/AdbControllerImp.kt b/src/main/kotlin/spock/adb/AdbControllerImp.kt index c432876..2ef2eea 100644 --- a/src/main/kotlin/spock/adb/AdbControllerImp.kt +++ b/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) -> Unit)? = null @@ -55,52 +55,72 @@ class AdbControllerImp( device: IDevice ) { - val activitiesList = mutableListOf() - val activitiesClass: List = GetBackStackCommand().execute(Any(), project, device) + // ADB must run on a background thread — wrap everything in execute {} + execute { + val activitiesList = mutableListOf() + val activitiesClass: List = 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, 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 - val activitiesClass: List = - 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 { _, 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 = + 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 { _, 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 { + 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 { + 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 + } } diff --git a/src/main/kotlin/spock/adb/SpockAdbViewer.kt b/src/main/kotlin/spock/adb/SpockAdbViewer.kt index 2adbe0c..23f8ba4 100755 --- a/src/main/kotlin/spock/adb/SpockAdbViewer.kt +++ b/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 -> diff --git a/src/main/kotlin/spock/adb/actions/BaseAction.kt b/src/main/kotlin/spock/adb/actions/BaseAction.kt index d08f7ea..c2e355a 100644 --- a/src/main/kotlin/spock/adb/actions/BaseAction.kt +++ b/src/main/kotlin/spock/adb/actions/BaseAction.kt @@ -11,12 +11,16 @@ 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) } @@ -24,8 +28,8 @@ abstract class BaseAction : AnAction() { 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, block: (device: IDevice) -> Unit) { diff --git a/src/main/kotlin/spock/adb/command/ConnectDeviceOverIPCommand.kt b/src/main/kotlin/spock/adb/command/ConnectDeviceOverIPCommand.kt index 8e528b7..d82e766 100644 --- a/src/main/kotlin/spock/adb/command/ConnectDeviceOverIPCommand.kt +++ b/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 { 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 "" } } diff --git a/src/main/kotlin/spock/adb/command/GetApplicationBackStackCommand.kt b/src/main/kotlin/spock/adb/command/GetApplicationBackStackCommand.kt index 2f2e6db..8840a00 100644 --- a/src/main/kotlin/spock/adb/command/GetApplicationBackStackCommand.kt +++ b/src/main/kotlin/spock/adb/command/GetApplicationBackStackCommand.kt @@ -9,11 +9,9 @@ import java.util.concurrent.TimeUnit class GetApplicationBackStackCommand : Command> { 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> { 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> { 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)) } } diff --git a/src/main/kotlin/spock/adb/command/GetApplicationPermission.kt b/src/main/kotlin/spock/adb/command/GetApplicationPermission.kt index 783d054..d6196c8 100644 --- a/src/main/kotlin/spock/adb/command/GetApplicationPermission.kt +++ b/src/main/kotlin/spock/adb/command/GetApplicationPermission.kt @@ -38,7 +38,7 @@ class GetApplicationPermission : Command> { } 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.") } diff --git a/src/main/kotlin/spock/adb/command/GetFragmentsCommand.kt b/src/main/kotlin/spock/adb/command/GetFragmentsCommand.kt index db75e88..e5be508 100644 --- a/src/main/kotlin/spock/adb/command/GetFragmentsCommand.kt +++ b/src/main/kotlin/spock/adb/command/GetFragmentsCommand.kt @@ -29,8 +29,9 @@ class GetFragmentsCommand : Command> { } private fun getFragmentsUsingOldMethod(bulkTaskDetails: String): List { - 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")) } diff --git a/src/main/kotlin/spock/adb/command/ProcessDeathCommand.kt b/src/main/kotlin/spock/adb/command/ProcessDeathCommand.kt index 829e9cc..cda94fb 100644 --- a/src/main/kotlin/spock/adb/command/ProcessDeathCommand.kt +++ b/src/main/kotlin/spock/adb/command/ProcessDeathCommand.kt @@ -14,7 +14,7 @@ class ProcessDeathCommand : Command { 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 { } } - 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) { diff --git a/src/main/kotlin/spock/adb/debugger/Debugger.kt b/src/main/kotlin/spock/adb/debugger/Debugger.kt index 7c0e293..0794fc8 100644 --- a/src/main/kotlin/spock/adb/debugger/Debugger.kt +++ b/src/main/kotlin/spock/adb/debugger/Debugger.kt @@ -23,7 +23,7 @@ class Debugger(private val project: Project, private val device: IDevice, privat } for (androidDebugger in AndroidDebugger.EP_NAME.extensions) { if (androidDebugger.supportsProject(project)) { - invokeLater { closeOldSessionAndRun(androidDebugger, device.getClient(packageName) ?: client!!) } + invokeLater { closeOldSessionAndRun(androidDebugger, device.getClient(packageName) ?: client ?: return@invokeLater) } break } } diff --git a/src/main/kotlin/spock/adb/premission/CheckBoxDialog.kt b/src/main/kotlin/spock/adb/premission/CheckBoxDialog.kt index c63fd8f..3f0410e 100644 --- a/src/main/kotlin/spock/adb/premission/CheckBoxDialog.kt +++ b/src/main/kotlin/spock/adb/premission/CheckBoxDialog.kt @@ -17,7 +17,7 @@ class CheckBoxDialog( setContentPane(contentPane) isModal = true prepareList() - defaultCloseOperation = WindowConstants.DISPOSE_ON_CLOSE + defaultCloseOperation = DISPOSE_ON_CLOSE addWindowListener(object : WindowAdapter() { override fun windowClosing(e: WindowEvent?) { onCancel() @@ -45,7 +45,6 @@ class CheckBoxDialog( handelSelection(event) } }) - // adaPermission.getApplicationPermissions() } private fun handelSelection(event: MouseEvent) {