From ea3b496b6d73829e52730b4b6cfdeb4d386f4109 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Wed, 2 Sep 2026 18:05:53 -0700 Subject: [PATCH 1/3] style: spotless reformat of JavaDebugAdapter, no functional change ADFA-5398 touches this file, which enrolls it in the `ratchetFrom = origin/stage` ratchet and reformats it in full - mostly brace-wrapping `when` branches and expression bodies. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011Crj29d6Q2DGjioWPxtuSR --- .../lsp/java/debug/JavaDebugAdapter.kt | 51 +++++++++++-------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/lsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/JavaDebugAdapter.kt b/lsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/JavaDebugAdapter.kt index a0ba24f34a..b8cc442082 100644 --- a/lsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/JavaDebugAdapter.kt +++ b/lsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/JavaDebugAdapter.kt @@ -146,7 +146,7 @@ internal class JavaDebugAdapter : _listenerState?.invalidate() listenerThread?.interrupt() - + _listenerState = ListenerState( client = client, @@ -154,19 +154,20 @@ internal class JavaDebugAdapter : args = args, ) - val failure = withContext(Dispatchers.IO) { - try { - logger.debug("startListening") - listenerState.startListening() - null - } catch (e: Throwable) { - if (e is CancellationException) { - throw e + val failure = + withContext(Dispatchers.IO) { + try { + logger.debug("startListening") + listenerState.startListening() + null + } catch (e: Throwable) { + if (e is CancellationException) { + throw e + } + logger.error("Failed to listen for incoming JDWP connections", e) + return@withContext DebugClientConnectionResult.Failure(cause = e) } - logger.error("Failed to listen for incoming JDWP connections", e) - return@withContext DebugClientConnectionResult.Failure(cause = e) } - } if (failure != null) { return failure @@ -354,7 +355,7 @@ internal class JavaDebugAdapter : val spec = when (breakpoint) { - is PositionalBreakpoint -> + is PositionalBreakpoint -> { specList.createBreakpoint( source = breakpoint.source, // +1 because we receive 0-indexed line numbers from the IDE @@ -363,8 +364,9 @@ internal class JavaDebugAdapter : qualifiedName = qualifiedName, suspendPolicy = breakpoint.suspendPolicy.asJdiInt(), ) + } - is MethodBreakpoint -> + is MethodBreakpoint -> { specList.createBreakpoint( source = breakpoint.source, methodId = breakpoint.methodId, @@ -372,8 +374,11 @@ internal class JavaDebugAdapter : qualifiedName = qualifiedName, suspendPolicy = breakpoint.suspendPolicy.asJdiInt(), ) + } - else -> throw IllegalArgumentException("Unsupported breakpoint type: $breakpoint") + else -> { + throw IllegalArgumentException("Unsupported breakpoint type: $breakpoint") + } } val result = @@ -385,19 +390,23 @@ internal class JavaDebugAdapter : val resolveSuccess = result.getOrDefault(false) when { - resolveSuccess && spec.isResolved -> + resolveSuccess && spec.isResolved -> { BreakpointResult.Success( breakpoint, false, ) + } - resolveSuccess && !spec.isResolved -> + resolveSuccess && !spec.isResolved -> { BreakpointResult.Success( breakpoint, true, ) + } - else -> BreakpointResult.Failure(breakpoint, failure) + else -> { + BreakpointResult.Failure(breakpoint, failure) + } } }, ) @@ -631,8 +640,10 @@ internal class JDWPListenerThread( override fun run() { logger.debug("run::start") if (!listenerState.isListening && !listenerState.isInvalidated) { - logger.warn("Listener should've been listening at this point, but it's not. " + - "Trying to start listening...") + logger.warn( + "Listener should've been listening at this point, but it's not. " + + "Trying to start listening...", + ) listenerState.startListening() } From b1b1d239bfab3e1fef0f735839b7d0f0bad5ffa7 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Wed, 2 Sep 2026 18:06:33 -0700 Subject: [PATCH 2/3] ADFA-5398: Release the debug client when the adapter closes JavaDebugAdapter.close() invalidated its ListenerState and interrupted the listener thread but never dropped the reference, so the retained chain JDWPListenerThread -> ListenerState -> IDebugClient -> DebuggerViewModel -> threads, frames, variablesTree stayed reachable. The adapter lives on JavaLanguageServer in the process-global registry, so nothing collected it until the next connectDebugClient replaced the state. close() now clears _listenerState and listenerThread in a finally, so the graph is released even if invalidate() or interrupt() throws. That runs on every project close: ProjectHandlerActivity calls destroyLanguageServers(), which reaches JavaLanguageServer.shutdown() and this close(). onConnectedToVm read _listenerState!! - a late callback from the listener thread would now NPE on the cleared field - so it takes the state into a local, logs and returns if the adapter has been closed underneath it. Unlike the thread leak in ADFA-5375, this one is a plain retained-object leak, so LeakCanary can see it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011Crj29d6Q2DGjioWPxtuSR --- .../lsp/java/debug/JavaDebugAdapter.kt | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/lsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/JavaDebugAdapter.kt b/lsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/JavaDebugAdapter.kt index b8cc442082..d2420c577e 100644 --- a/lsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/JavaDebugAdapter.kt +++ b/lsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/JavaDebugAdapter.kt @@ -238,7 +238,15 @@ internal class JavaDebugAdapter : threadState.initThreads() this.vms.add(vmConnection) - this._listenerState!!.client.onAttach(client) + + val state = this._listenerState + if (state == null) { + // close() ran while this connection was being established. + logger.warn("Connected to a VM after the debug adapter was closed; not attaching") + return + } + + state.client.onAttach(client) } override suspend fun connectedRemoteClients(): Set = vms.map(VmConnection::client).toSet() @@ -598,6 +606,13 @@ internal class JavaDebugAdapter : listenerThread?.interrupt() } catch (err: Throwable) { logger.error("Unable to stop VM connection listener", err) + } finally { + // ListenerState holds the IDebugClient, which holds the DebuggerViewModel and its whole + // thread/frame/variable state. This adapter lives on JavaLanguageServer in the global + // registry, so without dropping the reference here that graph stays reachable from a + // native GC root until the next connectDebugClient (ADFA-5398). + _listenerState = null + listenerThread = null } adapterScope.launch(Dispatchers.IO) { From ec66cedb523bd44a9096ef7cf386fa8600b73320 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Thu, 3 Sep 2026 13:46:19 -0700 Subject: [PATCH 3/3] ADFA-5398: Close the gaps the null-out opened Three follow-ups from review of the previous commit, all consequences of clearing _listenerState that I did not sweep for. connectDebugClient still read `_listenerState!!` when constructing the listener thread. Guarding onConnectedToVm and not this one was the same half-sweep the review has caught before: close() nulling the field from another thread makes that assertion throw. The state is now held in a local and passed from there, so the field can be nulled at any time without affecting the connect in flight. onConnectedToVm returned early on the closed path after it had already started the VM connection and added it to vms, leaving a live connection and its event handler attached to an adapter that was gone. It now removes and closes the connection before returning. close() ran invalidate() and interrupt() in one try, so a throw from the first skipped the second - and since JDWPListenerThread holds its own ListenerState reference, clearing the fields afterwards would release nothing. They are separate now, and the interrupt runs either way. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011Crj29d6Q2DGjioWPxtuSR --- .../lsp/java/debug/JavaDebugAdapter.kt | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/lsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/JavaDebugAdapter.kt b/lsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/JavaDebugAdapter.kt index d2420c577e..425bf343f9 100644 --- a/lsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/JavaDebugAdapter.kt +++ b/lsp/java/src/main/java/com/itsaky/androidide/lsp/java/debug/JavaDebugAdapter.kt @@ -147,12 +147,15 @@ internal class JavaDebugAdapter : _listenerState?.invalidate() listenerThread?.interrupt() - _listenerState = + // Held locally as well as in the field: close() may null the field from another thread, and + // re-reading it with !! below would then throw (ADFA-5398). + val state = ListenerState( client = client, connector = connector, args = args, ) + _listenerState = state val failure = withContext(Dispatchers.IO) { @@ -175,7 +178,7 @@ internal class JavaDebugAdapter : listenerThread = JDWPListenerThread( - _listenerState!!, + state, this::onConnectedToVm, ).also { thread -> thread.start() } return DebugClientConnectionResult.Success @@ -239,14 +242,19 @@ internal class JavaDebugAdapter : this.vms.add(vmConnection) - val state = this._listenerState - if (state == null) { - // close() ran while this connection was being established. - logger.warn("Connected to a VM after the debug adapter was closed; not attaching") + val listener = this._listenerState + if (listener == null) { + // close() ran while this connection was being established. Returning here without + // undoing the two lines above would leave a live VM connection, and its event handler, + // attached to an adapter that is gone. + logger.warn("Connected to a VM after the debug adapter was closed; dropping it") + vms.remove(vmConnection) + runCatching { vmConnection.close() } + .onFailure { err -> logger.error("Failed to close a VM connected after close()", err) } return } - state.client.onAttach(client) + listener.client.onAttach(client) } override suspend fun connectedRemoteClients(): Set = vms.map(VmConnection::client).toSet() @@ -601,8 +609,12 @@ internal class JavaDebugAdapter : override fun close() { logger.debug("close") + // Separate catches: the listener thread holds its own ListenerState reference, so if + // invalidate() throws and skips the interrupt, clearing the fields below releases nothing. + runCatching { _listenerState?.invalidate() } + .onFailure { err -> logger.error("Unable to invalidate the VM connection listener", err) } + try { - _listenerState?.invalidate() listenerThread?.interrupt() } catch (err: Throwable) { logger.error("Unable to stop VM connection listener", err)