ADFA-4128: ping the proxy app before calling a restart-deploy timeout an outdated runtime

awaitDisconnect only watched the connection registry, which clears when
linkToDeath fires. A loaded device can deliver that notification after the
5 s disconnect wait, and the deployer then reported a current runtime that had
exited as one that "predates restart support" and forced a multi-minute Gradle
rebuild.

On timeout the channel now pings the registered binder. A ping is a
transaction, so it fails as soon as the process is gone; the channel then clears
the registry as the late notification would and reports the runtime gone, and
the restart path proceeds to relaunch. A runtime that still answers keeps the
outdated-runtime verdict.

Review thread: #1718 (comment)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2

Author: fryanpan

quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/service/deploy/DeployChannel.kt

@@ -58,7 +58,8 @@ interface DeploySender {
 	 *
 	 * @param timeoutMillis upper bound on the wait, sized for a runtime exit rather than a
 	 *   process launch
-	 * @return true when disconnected within [timeoutMillis]
+	 * @return true when the runtime is gone within [timeoutMillis]: it disconnected, or its
+	 *   process no longer answers although the death notification has not landed yet
 	 */
 	suspend fun awaitDisconnect(timeoutMillis: Long): Boolean
 
@@ -227,14 +228,27 @@ class DeployChannel(
 		}
 	}
 
-	override suspend fun awaitDisconnect(timeoutMillis: Long): Boolean =
+	override suspend fun awaitDisconnect(timeoutMillis: Long): Boolean {
 		// The awaited value is null by construction, so the block must yield its own
 		// non-null sentinel: returning `first { it == null }` would make a real
 		// disconnect indistinguishable from a timeout.
-		withTimeoutOrNull(timeoutMillis) {
-			connections.target.first { it == null }
-			true
-		} == true
+		val disconnected =
+			withTimeoutOrNull(timeoutMillis) {
+				connections.target.first { it == null }
+				true
+			} == true
+		if (disconnected) return true
+		// The registry clears on linkToDeath, which a loaded device can deliver well after
+		// the process died. A ping is a transaction, so it fails as soon as the process is
+		// gone; a runtime that is genuinely still running answers it. Without this, a late
+		// notification reads as a runtime that ignored the restart and costs a rebuild.
+		val connection = connections.target.value ?: return false
+		val binder = connection.target.asBinder() ?: return false
+		if (binder.pingBinder()) return false
+		log.info("Proxy app process is gone; its death notification has not landed yet")
+		connections.onDisconnected(binder)
+		return true
+	}
 
 	override suspend fun awaitReconnect(timeoutMillis: Long): Long? =
 		withTimeoutOrNull(timeoutMillis) {

quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/service/deploy/PayloadDeployer.kt

@@ -188,9 +188,9 @@ internal class PayloadDeployer(
 		when (val result = recovered.result) {
 			is DeployResult.Reloaded -> {
 				if (!deploy.awaitDisconnect(restartDisconnectTimeoutMillis)) {
-					// The runtime acked but kept running, so it predates restart support
-					// and hot-swapped instead, leaving a live service possibly stale. A
-					// proxy app rebuild reinstalls a current runtime.
+					// The runtime acked but its process still answers, so it predates
+					// restart support and hot-swapped instead, leaving a live service
+					// possibly stale. A proxy app rebuild reinstalls a current runtime.
 					return BuildOutcome.RequiresProxyAppRebuild(
 						InvalidationReason.OUTDATED_BASELINE,
 						"proxy app acknowledged a restart deploy but did not exit " +

quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/service/deploy/DeployChannelWaitsTest.kt

@@ -40,6 +40,42 @@ class DeployChannelWaitsTest {
 
 	private fun connect(generation: Long) = connections.onConnected(ConnectedTarget(target, "com.example.quickbuild", generation))
 
+	/**
+	 * A target whose binder answers [IBinder.pingBinder] as scripted. A reflection proxy,
+	 * because android.jar's [IBinder] is unmocked on the JVM and only the ping is live.
+	 */
+	private fun pingable(alive: Boolean): PingableTarget = PingableTarget(alive)
+
+	private class PingableTarget(
+		alive: Boolean,
+	) {
+		private val binder: IBinder =
+			java.lang.reflect.Proxy.newProxyInstance(
+				IBinder::class.java.classLoader,
+				arrayOf(IBinder::class.java),
+			) { _, method, _ ->
+				when (method.name) {
+					"pingBinder" -> alive
+					else -> null
+				}
+			} as IBinder
+
+		val target =
+			object : IQuickBuildTarget {
+				override fun asBinder(): IBinder = binder
+
+				override fun onPayload(
+					generation: Long,
+					dexPayload: ParcelFileDescriptor?,
+					resourcesPayload: ParcelFileDescriptor?,
+					assetsPayload: ParcelFileDescriptor?,
+					metadataJson: String?,
+				) = Unit
+
+				override fun onBuildStatus(statusJson: String?) = Unit
+			}
+	}
+
 	@Test
 	fun `awaitDisconnect reports true when the proxy app actually disconnects`() =
 		runTest {
@@ -69,6 +105,35 @@ class DeployChannelWaitsTest {
 			assertThat(channel.awaitDisconnect(5_000)).isTrue()
 		}
 
+	@Test
+	fun `awaitDisconnect reports true on timeout when the process no longer answers a ping`() =
+		runTest {
+			// The death notification is late, not absent: the process is gone, so a ping
+			// fails. Reporting false here would attribute the timeout to a runtime that
+			// ignored the restart and cost a Gradle rebuild.
+			val dead = pingable(alive = false)
+			connections.onConnected(ConnectedTarget(dead.target, "com.example.quickbuild", 7))
+			val awaited = async { channel.awaitDisconnect(5_000) }
+
+			advanceTimeBy(5_001)
+
+			assertThat(awaited.await()).isTrue()
+			assertThat(connections.target.value).isNull()
+		}
+
+	@Test
+	fun `awaitDisconnect reports false on timeout when the process still answers a ping`() =
+		runTest {
+			val alive = pingable(alive = true)
+			connections.onConnected(ConnectedTarget(alive.target, "com.example.quickbuild", 7))
+			val awaited = async { channel.awaitDisconnect(5_000) }
+
+			advanceTimeBy(5_001)
+
+			assertThat(awaited.await()).isFalse()
+			assertThat(connections.target.value).isNotNull()
+		}
+
 	@Test
 	fun `awaitReconnect returns the generation the fresh process reported`() =
 		runTest {

quickbuild/docs/debugging.md

@@ -337,7 +337,7 @@ not, and changing those means editing the constant: `MODULE_SCAN_MAX_DEPTH`, `UI
 | Daemon request timeout | `DaemonProcessClient.DEFAULT_REQUEST_TIMEOUT_MILLIS` | 300 s | Per-request ceiling. Exceeding it fails that request and releases the slot; it does not by itself count as daemon death. |
 | Daemon shutdown grace | `DaemonProcessClient.SHUTDOWN_TIMEOUT_MILLIS` | 3 s | How long a polite `shutdown` is given before the child is killed. |
 | Deploy round trip | `DeployChannel.DEFAULT_TIMEOUT_MILLIS` | 15 s | One AIDL `onPayload` call. Exceeding it fails the deploy. |
-| Restart-deploy disconnect wait | `LiveReloadExecutorImpl.DEFAULT_RESTART_DISCONNECT_TIMEOUT_MILLIS` | 5 s | How long the host waits for the proxy app to exit after a restart deploy. A runtime that acked but kept running is treated as an outdated baseline and forces a proxy app rebuild. |
+| Restart-deploy disconnect wait | `LiveReloadExecutorImpl.DEFAULT_RESTART_DISCONNECT_TIMEOUT_MILLIS` | 5 s | How long the host waits for the proxy app to exit after a restart deploy. On timeout the host pings the binder, so a late death notification is not mistaken for a running app; a runtime that acked and still answers is treated as an outdated baseline and forces a proxy app rebuild. |
 | Restart-deploy reconnect wait | `LiveReloadExecutorImpl.DEFAULT_RESTART_RECONNECT_TIMEOUT_MILLIS` | 15 s | How long the host waits for the relaunched proxy app to rebind. |
 | Runtime rebind backoff floor | `QuickBuildClient.REBIND_MIN_DELAY_MS` | 1 s | First rebind delay inside the proxy app, doubled per failed attempt and reset on every successful connect. |
 | Runtime rebind backoff ceiling | `QuickBuildClient.REBIND_MAX_DELAY_MS` | 30 s | Ceiling for that doubling, so a CoGo that never comes back costs one attempt per 30 s. |