ADFA-5659: shut the Gradle daemon watcher down with the server

subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcher.kt

@@ -124,8 +124,34 @@ internal class GradleDaemonWatcher(
 			.onFailure { err -> log.warn("Failed to report exit of Gradle daemon {}", pid, err) }
 	}
 
+	/**
+	 * Stops the poller.
+	 *
+	 * Graceful first, so a report already queued -- an exit picked up moments before the server was
+	 * told to stop -- still runs; then forcefully, so a poll asleep between attempts cannot hold the
+	 * process open. [SHUTDOWN_GRACE_MS] is the bound: work is one `ProcessHandle.descendants()`
+	 * scan, not a build, so a poll that has not finished in that long is wedged rather than busy.
+	 *
+	 * A report that has *not* been submitted yet is lost, and that is accepted: it arrives via
+	 * onExit on a process-reaper thread once the OS reaps the daemon, which at shutdown is after
+	 * everything here has run.
+	 */
 	fun shutdown() {
-		scheduler.shutdownNow()
+		scheduler.shutdown()
+		val drained =
+			runCatching { scheduler.awaitTermination(SHUTDOWN_GRACE_MS, TimeUnit.MILLISECONDS) }
+				.onFailure { err ->
+					// Only an interrupt is restored. Anything else out of awaitTermination says
+					// nothing about this thread's cancellation state, and marking it interrupted
+					// would abort the caller's next blocking call -- ToolingApiServerImpl.shutdown's
+					// wait on the connection close -- over a failure unrelated to it.
+					if (err is InterruptedException) {
+						Thread.currentThread().interrupt()
+					}
+				}.getOrDefault(false)
+		if (!drained) {
+			scheduler.shutdownNow()
+		}
 	}
 
 	companion object {
@@ -143,6 +169,9 @@ internal class GradleDaemonWatcher(
 
 		private const val POLL_INTERVAL_MS = 500L
 
+		/** How long [shutdown] lets queued reports finish before it stops waiting. */
+		const val SHUTDOWN_GRACE_MS = 250L
+
 		/** Bounded at roughly a minute, which is far longer than a daemon takes to come up. */
 		const val MAX_POLL_ATTEMPTS = 120
 

subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/ToolingApiServerImpl.kt

@@ -73,7 +73,10 @@ import kotlin.concurrent.withLock
  *
  * @author Akash Yadav
  */
-internal class ToolingApiServerImpl : IToolingApiServer {
+internal class ToolingApiServerImpl(
+	private val newDaemonWatcher: (onStarted: (Int) -> Unit, onExited: (Int) -> Unit) -> GradleDaemonWatcher =
+		{ onStarted, onExited -> GradleDaemonWatcher(onStarted = onStarted, onExited = onExited) },
+) : IToolingApiServer {
 	private var client: IToolingApiClient? = null
 	private var connector: GradleConnector? = null
 	private var connection: ProjectConnection? = null
@@ -361,11 +364,45 @@ internal class ToolingApiServerImpl : IToolingApiServer {
 	 * Finds the Gradle daemon and reports it to the client, so the memory chart can plot the process
 	 * that actually holds the build's heap (ADFA-5514).
 	 */
-	private val daemonWatcher by lazy {
-		GradleDaemonWatcher(
-			onStarted = { pid -> client?.onGradleDaemonStarted(pid) },
-			onExited = { pid -> client?.onGradleDaemonExited(pid) },
-		)
+	private val lazyDaemonWatcher =
+		lazy {
+			newDaemonWatcher(
+				{ pid -> client?.onGradleDaemonStarted(pid) },
+				{ pid -> client?.onGradleDaemonExited(pid) },
+			)
+		}
+
+	private val daemonWatcher by lazyDaemonWatcher
+
+	/**
+	 * Serialises constructing the watcher against stopping it.
+	 *
+	 * `shutdown` and `executeTasks` arrive as separate requests and run their bodies on the common
+	 * pool, so a build submitted just before a shutdown can reach [startDaemonWatch] after shutdown
+	 * has already asked whether a watcher exists. Without this, that build constructs a watcher, and
+	 * its scheduler, that nothing will ever stop -- the leak this class's shutdown call exists to
+	 * prevent, arriving by the one route the `isInitialized` check cannot see.
+	 */
+	private val daemonWatcherLock = Any()
+
+	/** Guarded by [daemonWatcherLock]. */
+	private var isDaemonWatcherShutdown = false
+
+	/**
+	 * Starts a daemon search for a build that is beginning, unless the server is shutting down.
+	 *
+	 * Only the construction is locked. `onBuildStarted` runs outside it, so a watcher stopped in
+	 * between hits the rejection its own guard already handles rather than blocking a build thread.
+	 */
+	private fun startDaemonWatch() {
+		val watcher =
+			synchronized(daemonWatcherLock) {
+				if (isDaemonWatcherShutdown) {
+					return
+				}
+				daemonWatcher
+			}
+		watcher.onBuildStarted()
 	}
 
 	private fun notifyBuildFailure(result: BuildResult) {
@@ -407,6 +444,37 @@ internal class ToolingApiServerImpl : IToolingApiServer {
 			buildCancellationToken?.cancel()
 			buildCancellationToken = null
 
+			// Early, and deliberately not "late enough to report the daemon's exit".
+			//
+			// The leaked thread is the defect: unstopped, an in-flight poll chain goes on scanning
+			// ProcessHandle.descendants() for up to a minute after the server is gone. Stopping it
+			// here ends that at once, and means no later poll can report a daemon into an RPC
+			// channel that is being torn down.
+			//
+			// Delivering the shutdown-time exit was tried and does not work. That report arrives
+			// through handle.onExit().thenRun { scheduler.execute { ... } }, and onExit completes
+			// on a process-reaper thread only once the OS has reaped the daemon -- strictly after
+			// DefaultGradleConnector.close() returns. There is no point in this sequence where the
+			// scheduler is still accepting work *and* the daemon has already been reaped, so the
+			// report is not deliverable at shutdown whatever the ordering; keeping `client` alive
+			// for it only widens the window in which a half-torn-down channel can be written to.
+			// The client learns the daemon is gone when it reconnects, not from here.
+			//
+			// Through the lazy delegate rather than the property: touching the property would
+			// construct a watcher, and its scheduler, only to shut it down again on a server that
+			// never ran a build. The flag closes the other half of that: a build that reaches
+			// startDaemonWatch after this point must not build one either. See daemonWatcherLock.
+			val watcher =
+				synchronized(daemonWatcherLock) {
+					isDaemonWatcherShutdown = true
+					if (lazyDaemonWatcher.isInitialized()) daemonWatcher else null
+				}
+			if (watcher != null) {
+				log.info("Stopping the Gradle daemon watcher...")
+				runCatching { watcher.shutdown() }
+					.onFailure { log.warn("Could not stop the Gradle daemon watcher", it) }
+			}
+
 			val connection = this.connection
 			val connector = this.connector
 			this.connection = null
@@ -468,7 +536,7 @@ internal class ToolingApiServerImpl : IToolingApiServer {
 			}
 
 			isBuildInProgress = true
-			daemonWatcher.onBuildStarted()
+			startDaemonWatch()
 			try {
 				action()
 			} finally {

subprojects/tooling-api-impl/src/test/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcherTest.kt

@@ -202,6 +202,34 @@ class GradleDaemonWatcherTest {
 		}
 	}
 
+	@Test
+	fun `a wait that fails for another reason does not mark the thread interrupted`() {
+		// shutdown() runs as a teardown step on a shared pool worker, and its caller's next act is
+		// a blocking wait on the connection close. A flag set for a failure that has nothing to do
+		// with cancellation aborts that wait, so the connector teardown is never waited on.
+		val scheduler = mockk<ScheduledExecutorService>(relaxed = true)
+		every { scheduler.awaitTermination(any(), any()) } throws IllegalStateException("not an interrupt")
+
+		watcher(scheduler = scheduler).shutdown()
+
+		// Reads and clears, so a stray flag cannot leak into the tests that follow either.
+		assertThat(Thread.interrupted()).isFalse()
+		// The failure still counts as "not drained", so the forceful stop still runs.
+		verify(exactly = 1) { scheduler.shutdownNow() }
+	}
+
+	@Test
+	fun `an interrupted wait still marks the thread interrupted`() {
+		// The other side of the narrowing: a real interrupt is a cancellation request and is not
+		// this class's to swallow.
+		val scheduler = mockk<ScheduledExecutorService>(relaxed = true)
+		every { scheduler.awaitTermination(any(), any()) } throws InterruptedException()
+
+		watcher(scheduler = scheduler).shutdown()
+
+		assertThat(Thread.interrupted()).isTrue()
+	}
+
 	private companion object {
 		/** What the daemon's command line looks like on device, trimmed to the identifying part. */
 		const val DAEMON_COMMAND_LINE =
@@ -213,4 +241,28 @@ class GradleDaemonWatcherTest {
 		/** Every poll attempt, plus the initial schedule. */
 		const val MAX_SCHEDULES = GradleDaemonWatcher.MAX_POLL_ATTEMPTS + 1
 	}
+
+	@Test
+	fun `shutdown drains what is queued before it stops waiting`() {
+		// Graceful first: a report queued moments before the server was told to stop still runs.
+		val scheduler = mockk<ScheduledExecutorService>(relaxed = true)
+		every { scheduler.awaitTermination(any(), any()) } returns true
+
+		watcher(scheduler = scheduler).shutdown()
+
+		verify(exactly = 1) { scheduler.shutdown() }
+		verify(exactly = 1) { scheduler.awaitTermination(GradleDaemonWatcher.SHUTDOWN_GRACE_MS, TimeUnit.MILLISECONDS) }
+		verify(exactly = 0) { scheduler.shutdownNow() }
+	}
+
+	@Test
+	fun `shutdown stops waiting on a poll that will not finish`() {
+		// And forcefully after the grace period, so a wedged scan cannot hold the process open.
+		val scheduler = mockk<ScheduledExecutorService>(relaxed = true)
+		every { scheduler.awaitTermination(any(), any()) } returns false
+
+		watcher(scheduler = scheduler).shutdown()
+
+		verify(exactly = 1) { scheduler.shutdownNow() }
+	}
 }

subprojects/tooling-api-impl/src/test/java/com/itsaky/androidide/tooling/impl/ToolingApiServerImplTest.kt

@@ -15,11 +15,14 @@ import io.mockk.spyk
 import io.mockk.verify
 import org.gradle.tooling.GradleConnector
 import org.gradle.tooling.ProjectConnection
+import org.junit.Assert.fail
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
 import java.io.File
+import java.util.concurrent.CountDownLatch
 import java.util.concurrent.TimeUnit
+import java.util.concurrent.TimeoutException
 
 /**
  * @author Akash Yadav
@@ -124,4 +127,97 @@ class ToolingApiServerImplTest {
 			RootModelBuilder.build(initParams, any())
 		}
 	}
+
+	@Test
+	fun `shutting the server down stops the daemon watcher`() {
+		// The defect this PR exists to fix, pinned at the caller. The watcher's own shutdown() was
+		// already correct on stage -- what was missing was anything calling it, so a test of
+		// GradleDaemonWatcher.shutdown() in isolation passes against the unfixed server and pins
+		// nothing. Deleting the block in ToolingApiServerImpl.shutdown() has to fail a test.
+		val watcher = mockk<GradleDaemonWatcher>(relaxed = true)
+		val server = ToolingApiServerImpl(newDaemonWatcher = { _, _ -> watcher })
+
+		// A build, to bring the watcher into being the way a session does: runBuild calls
+		// onBuildStarted, which is what initializes the lazy.
+		server.initialize(testInitParams()).get(5, TimeUnit.SECONDS)
+		server.shutdown().get(5, TimeUnit.SECONDS)
+
+		verify(exactly = 1) { watcher.shutdown() }
+	}
+
+	@Test
+	fun `a server that never ran a build does not build a watcher just to stop it`() {
+		// Through the lazy delegate, not the property: touching the property would construct a
+		// watcher, and its scheduler thread, only to shut it down again.
+		var built = 0
+		val server =
+			ToolingApiServerImpl(
+				newDaemonWatcher = { _, _ ->
+					built++
+					mockk(relaxed = true)
+				},
+			)
+
+		server.shutdown().get(5, TimeUnit.SECONDS)
+
+		assertThat(built).isEqualTo(0)
+	}
+
+	@Test
+	fun `a build that starts after shutdown does not build a watcher`() {
+		// Half of the shutdown-versus-build race. A build request already in flight runs its body on
+		// the common pool, so it can reach startDaemonWatch after shutdown has looked for a watcher
+		// and found none. Building one here leaves a scheduler nothing will ever stop.
+		var built = 0
+		val server =
+			ToolingApiServerImpl(
+				newDaemonWatcher = { _, _ ->
+					built++
+					mockk(relaxed = true)
+				},
+			)
+
+		server.shutdown().get(5, TimeUnit.SECONDS)
+		server.initialize(testInitParams()).get(5, TimeUnit.SECONDS)
+
+		assertThat(built).isEqualTo(0)
+	}
+
+	@Test
+	fun `shutdown waits for a watcher a concurrent build is building`() {
+		// The other half: the build gets there first, and is still inside the constructor when
+		// shutdown asks. `lazy.isInitialized()` reads false throughout that window, so without the
+		// lock shutdown concludes there is no watcher and returns while one is being built.
+		//
+		// The negative assertion is bounded rather than exact: it proves shutdown did not conclude
+		// within a second, against an unlocked shutdown that runs in milliseconds.
+		val watcher = mockk<GradleDaemonWatcher>(relaxed = true)
+		val constructing = CountDownLatch(1)
+		val release = CountDownLatch(1)
+		val server =
+			ToolingApiServerImpl(
+				newDaemonWatcher = { _, _ ->
+					constructing.countDown()
+					assertThat(release.await(5, TimeUnit.SECONDS)).isTrue()
+					watcher
+				},
+			)
+
+		val build = server.initialize(testInitParams())
+		assertThat(constructing.await(5, TimeUnit.SECONDS)).isTrue()
+
+		val shutdown = server.shutdown()
+		try {
+			shutdown.get(1, TimeUnit.SECONDS)
+			fail("shutdown completed while the watcher was still being constructed")
+		} catch (_: TimeoutException) {
+			// expected: shutdown is waiting on the lock the build holds
+		}
+
+		release.countDown()
+		build.get(5, TimeUnit.SECONDS)
+		shutdown.get(5, TimeUnit.SECONDS)
+
+		verify(exactly = 1) { watcher.shutdown() }
+	}
 }