From 4b828a159c364e3ff122123a413c566d7f83e89f Mon Sep 17 00:00:00 2001 From: NathanFallet Date: Wed, 26 Aug 2026 19:41:41 +0200 Subject: [PATCH 1/3] fix: wait for the browser to actually exit when stopping it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit stop() called process.destroy() and moved on. destroy() only requests termination — it returns immediately, and it does not reach the browser's child processes. On Windows the renderer and GPU children outlive it and keep open handles on the profile directory. A browser started on the same --user-data-dir shortly after therefore cannot take ownership of the profile: it hangs before opening its debug port, and the start times out after the full connect window with no visible cause. Observed in production as a restart loop — close, reopen one second later, fail for 30s, repeat — that only a reboot could clear. stop() now uses destroyAndAwaitExit(): grace period after destroy(), escalation to killTree(), then a second wait. Same shape as zendriver's Browser.stop, which terminates, polls for up to 3 seconds, kills, then waits. The waiting loop is common code since isAlive() exists on every target; only the forceful kill is per-platform. On JVM killTree() also takes down descendants, snapshotting them first because killing the parent detaches them. On POSIX it sends SIGKILL to the process alone, which is enough there — unlike on Windows, children do not outlive their parent. --- .../kdriver/core/browser/DefaultBrowser.kt | 6 ++- .../dev/kdriver/core/browser/Process.kt | 48 +++++++++++++++++++ .../dev/kdriver/core/browser/Process.jvm.kt | 13 +++++ .../dev/kdriver/core/browser/Process.posix.kt | 11 +++++ 4 files changed, 77 insertions(+), 1 deletion(-) diff --git a/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt b/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt index 0dfe5ca3a..2aab87f52 100644 --- a/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt +++ b/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt @@ -362,7 +362,11 @@ open class DefaultBrowser( logger.debug("Closing browser gracefully...") withTimeoutOrNull(5.seconds) { connection?.browser?.close() } logger.debug("Killing browser process...") - process?.destroy() + // Wait for it to actually be gone, don't just ask. destroy() returns immediately and does not + // reach the browser's child processes, which keep open handles on the profile directory — + // a browser started on the same --user-data-dir right after would hang before opening its + // debug port, and the start would time out for no visible reason. + process?.let { if (!it.destroyAndAwaitExit()) logger.warn("Browser process ${it.pid()} still alive after kill") } process = null logger.debug("Closing connection...") connection?.close() diff --git a/core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt b/core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt index 3be6d121a..4a23e9d15 100644 --- a/core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt +++ b/core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt @@ -1,5 +1,6 @@ package dev.kdriver.core.browser +import kotlinx.coroutines.delay import kotlinx.io.files.Path expect abstract class Process { @@ -21,6 +22,53 @@ expect suspend fun Process.readStderrSnapshot( timeoutMillis: Long = 250, ): String? +/** + * Forcibly kills the process and, where the platform supports it, its whole tree. + * + * [Process.destroy] only asks politely (SIGTERM / TerminateProcess on the top-level process). On + * Windows in particular, a browser's renderer and GPU children survive it. + */ +expect fun Process.killTree() + +/** + * Terminates the process, then waits until it has actually gone. + * + * [Process.destroy] only *requests* termination: it returns immediately, and it does not reach the + * browser's child processes. Those children keep open handles on the profile directory, so a browser + * started on the same `--user-data-dir` shortly afterwards cannot take ownership of it: it hangs + * before opening its debug port, and the start times out with no visible cause. Callers therefore + * need to know the browser is really gone, not merely that it was asked to leave. + * + * Grace-waits for [gracePeriodMillis], then escalates to [killTree] and waits up to + * [killTimeoutMillis] more. The same shape as zendriver's `Browser.stop`. + * + * @return true if the process exited, false if it was still alive when the timeouts elapsed. + */ +suspend fun Process.destroyAndAwaitExit( + gracePeriodMillis: Long = 3_000, + killTimeoutMillis: Long = 5_000, +): Boolean { + if (!isAlive()) return true + + destroy() + if (awaitExit(gracePeriodMillis)) return true + + killTree() + return awaitExit(killTimeoutMillis) +} + +private suspend fun Process.awaitExit(timeoutMillis: Long): Boolean { + var waited = 0L + while (waited < timeoutMillis) { + if (!isAlive()) return true + delay(EXIT_POLL_INTERVAL_MILLIS) + waited += EXIT_POLL_INTERVAL_MILLIS + } + return !isAlive() +} + +private const val EXIT_POLL_INTERVAL_MILLIS = 100L + expect suspend fun startProcess(exe: Path, params: List): Process expect fun addShutdownHook(hook: suspend () -> Unit) expect fun isPosix(): Boolean diff --git a/core/src/jvmMain/kotlin/dev/kdriver/core/browser/Process.jvm.kt b/core/src/jvmMain/kotlin/dev/kdriver/core/browser/Process.jvm.kt index d88df4231..ff6dda8c7 100644 --- a/core/src/jvmMain/kotlin/dev/kdriver/core/browser/Process.jvm.kt +++ b/core/src/jvmMain/kotlin/dev/kdriver/core/browser/Process.jvm.kt @@ -92,6 +92,19 @@ actual suspend fun Process.readStderrSnapshot(maxBytes: Int, timeoutMillis: Long } } +/** + * Kills this process and every descendant. + * + * `Process.destroyForcibly()` on its own only reaches the top-level process; on Windows the browser's + * renderer and GPU children outlive it and keep holding the profile's files. Descendants are snapshot + * first, because killing the parent detaches them and they can no longer be enumerated. + */ +actual fun Process.killTree() { + val descendants = runCatching { toHandle().descendants().toList() }.getOrDefault(emptyList()) + destroyForcibly() + descendants.forEach { runCatching { it.destroyForcibly() } } +} + actual fun freePort(): Int? { ServerSocket(0, 5, InetAddress.getByName("127.0.0.1")).use { socket -> return socket.localPort diff --git a/core/src/posixMain/kotlin/dev/kdriver/core/browser/Process.posix.kt b/core/src/posixMain/kotlin/dev/kdriver/core/browser/Process.posix.kt index 0aa3fba46..9834ec3c0 100644 --- a/core/src/posixMain/kotlin/dev/kdriver/core/browser/Process.posix.kt +++ b/core/src/posixMain/kotlin/dev/kdriver/core/browser/Process.posix.kt @@ -22,6 +22,17 @@ actual abstract class Process { actual suspend fun Process.readStderrSnapshot(maxBytes: Int, timeoutMillis: Long): String? = null +/** + * Kills this process with SIGKILL. + * + * POSIX has no cheap, portable way to enumerate descendants, so only the process itself is signalled. + * That is enough here: on Linux and macOS the browser's children terminate with their parent, unlike + * on Windows. + */ +actual fun Process.killTree() { + if (isAlive()) kill(processIdentifier, SIGKILL) +} + actual fun addShutdownHook(hook: suspend () -> Unit) { // POSIX doesn't have a direct equivalent to Java shutdown hooks // Could use atexit() but it doesn't support suspend functions From efac0b6c3dd5b4af9193dfc9460f2ba0ae596035 Mon Sep 17 00:00:00 2001 From: NathanFallet Date: Wed, 26 Aug 2026 20:18:51 +0200 Subject: [PATCH 2/3] fix: provide killTree on the mingw and js targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Windows native and JS source sets had no actual, so :core:compileKotlinMingwX64 failed with 'Expected killTree has no actual declaration'. Only the JVM and POSIX ones had been written. On mingw, TerminateProcess already ends the process outright, so there is nothing gentler to escalate from. It does not reach the children: enumerating those on Windows means walking a CreateToolhelp32Snapshot by parent id, which this target does not do. That gap is documented rather than papered over — the JVM target is the one running browsers in production, and it uses ProcessHandle.descendants(). On JS it throws UnsupportedOperationException, like every other process function there. --- .../kotlin/dev/kdriver/core/browser/Process.js.kt | 4 ++++ .../dev/kdriver/core/browser/Process.mingw.kt | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/core/src/jsMain/kotlin/dev/kdriver/core/browser/Process.js.kt b/core/src/jsMain/kotlin/dev/kdriver/core/browser/Process.js.kt index cca0ef77c..af1e2b93c 100644 --- a/core/src/jsMain/kotlin/dev/kdriver/core/browser/Process.js.kt +++ b/core/src/jsMain/kotlin/dev/kdriver/core/browser/Process.js.kt @@ -18,6 +18,10 @@ actual suspend fun Process.readStderrSnapshot(maxBytes: Int, timeoutMillis: Long throw UnsupportedOperationException() } +actual fun Process.killTree() { + throw UnsupportedOperationException() +} + actual suspend fun startProcess( exe: Path, params: List, diff --git a/core/src/mingwMain/kotlin/dev/kdriver/core/browser/Process.mingw.kt b/core/src/mingwMain/kotlin/dev/kdriver/core/browser/Process.mingw.kt index f340d642d..3c86da365 100644 --- a/core/src/mingwMain/kotlin/dev/kdriver/core/browser/Process.mingw.kt +++ b/core/src/mingwMain/kotlin/dev/kdriver/core/browser/Process.mingw.kt @@ -53,6 +53,19 @@ private class WindowsProcess( */ actual suspend fun Process.readStderrSnapshot(maxBytes: Int, timeoutMillis: Long): String? = null +/** + * Kills this process. + * + * `TerminateProcess` already ends it outright, so there is nothing gentler to escalate from here. + * Note that it does **not** reach the process's children: enumerating them on Windows means walking + * a `CreateToolhelp32Snapshot` by parent id, which this target does not do — the JVM target, which is + * the one running browsers in production, uses `ProcessHandle.descendants()` for that. + */ +@OptIn(ExperimentalForeignApi::class) +actual fun Process.killTree() { + processHandle?.let { if (isAlive()) TerminateProcess(it, 1u) } +} + @OptIn(ExperimentalForeignApi::class) actual suspend fun startProcess( exe: Path, From c7fbfa2cb8e27d33d71d3dbc81322daeedad3c8b Mon Sep 17 00:00:00 2001 From: NathanFallet Date: Wed, 26 Aug 2026 20:23:21 +0200 Subject: [PATCH 3/3] style: keep the exit helpers out of Process.kt Moves killTree, destroyAndAwaitExit and its polling helper to their own file: Process.kt was over the per-file function threshold once both killTree and readStderrSnapshot were added. Also folds one early return and wraps a long line. No behaviour change. Lint issues on these files are back to the same count as main. --- .../kdriver/core/browser/DefaultBrowser.kt | 4 +- .../dev/kdriver/core/browser/Process.kt | 48 ------------------ .../dev/kdriver/core/browser/ProcessExit.kt | 50 +++++++++++++++++++ 3 files changed, 53 insertions(+), 49 deletions(-) create mode 100644 core/src/commonMain/kotlin/dev/kdriver/core/browser/ProcessExit.kt diff --git a/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt b/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt index 2aab87f52..e211166e2 100644 --- a/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt +++ b/core/src/commonMain/kotlin/dev/kdriver/core/browser/DefaultBrowser.kt @@ -366,7 +366,9 @@ open class DefaultBrowser( // reach the browser's child processes, which keep open handles on the profile directory — // a browser started on the same --user-data-dir right after would hang before opening its // debug port, and the start would time out for no visible reason. - process?.let { if (!it.destroyAndAwaitExit()) logger.warn("Browser process ${it.pid()} still alive after kill") } + process?.let { + if (!it.destroyAndAwaitExit()) logger.warn("Browser process ${it.pid()} still alive after kill") + } process = null logger.debug("Closing connection...") connection?.close() diff --git a/core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt b/core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt index 4a23e9d15..3be6d121a 100644 --- a/core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt +++ b/core/src/commonMain/kotlin/dev/kdriver/core/browser/Process.kt @@ -1,6 +1,5 @@ package dev.kdriver.core.browser -import kotlinx.coroutines.delay import kotlinx.io.files.Path expect abstract class Process { @@ -22,53 +21,6 @@ expect suspend fun Process.readStderrSnapshot( timeoutMillis: Long = 250, ): String? -/** - * Forcibly kills the process and, where the platform supports it, its whole tree. - * - * [Process.destroy] only asks politely (SIGTERM / TerminateProcess on the top-level process). On - * Windows in particular, a browser's renderer and GPU children survive it. - */ -expect fun Process.killTree() - -/** - * Terminates the process, then waits until it has actually gone. - * - * [Process.destroy] only *requests* termination: it returns immediately, and it does not reach the - * browser's child processes. Those children keep open handles on the profile directory, so a browser - * started on the same `--user-data-dir` shortly afterwards cannot take ownership of it: it hangs - * before opening its debug port, and the start times out with no visible cause. Callers therefore - * need to know the browser is really gone, not merely that it was asked to leave. - * - * Grace-waits for [gracePeriodMillis], then escalates to [killTree] and waits up to - * [killTimeoutMillis] more. The same shape as zendriver's `Browser.stop`. - * - * @return true if the process exited, false if it was still alive when the timeouts elapsed. - */ -suspend fun Process.destroyAndAwaitExit( - gracePeriodMillis: Long = 3_000, - killTimeoutMillis: Long = 5_000, -): Boolean { - if (!isAlive()) return true - - destroy() - if (awaitExit(gracePeriodMillis)) return true - - killTree() - return awaitExit(killTimeoutMillis) -} - -private suspend fun Process.awaitExit(timeoutMillis: Long): Boolean { - var waited = 0L - while (waited < timeoutMillis) { - if (!isAlive()) return true - delay(EXIT_POLL_INTERVAL_MILLIS) - waited += EXIT_POLL_INTERVAL_MILLIS - } - return !isAlive() -} - -private const val EXIT_POLL_INTERVAL_MILLIS = 100L - expect suspend fun startProcess(exe: Path, params: List): Process expect fun addShutdownHook(hook: suspend () -> Unit) expect fun isPosix(): Boolean diff --git a/core/src/commonMain/kotlin/dev/kdriver/core/browser/ProcessExit.kt b/core/src/commonMain/kotlin/dev/kdriver/core/browser/ProcessExit.kt new file mode 100644 index 000000000..8101d3dce --- /dev/null +++ b/core/src/commonMain/kotlin/dev/kdriver/core/browser/ProcessExit.kt @@ -0,0 +1,50 @@ +package dev.kdriver.core.browser + +import kotlinx.coroutines.delay + +/** + * Forcibly kills the process and, where the platform supports it, its whole tree. + * + * [Process.destroy] only asks politely (SIGTERM / TerminateProcess on the top-level process). On + * Windows in particular, a browser's renderer and GPU children survive it. + */ +expect fun Process.killTree() + +/** + * Terminates the process, then waits until it has actually gone. + * + * [Process.destroy] only *requests* termination: it returns immediately, and it does not reach the + * browser's child processes. Those children keep open handles on the profile directory, so a browser + * started on the same `--user-data-dir` shortly afterwards cannot take ownership of it: it hangs + * before opening its debug port, and the start times out with no visible cause. Callers therefore + * need to know the browser is really gone, not merely that it was asked to leave. + * + * Grace-waits for [gracePeriodMillis], then escalates to [killTree] and waits up to + * [killTimeoutMillis] more. The same shape as zendriver's `Browser.stop`. + * + * @return true if the process exited, false if it was still alive when the timeouts elapsed. + */ +suspend fun Process.destroyAndAwaitExit( + gracePeriodMillis: Long = 3_000, + killTimeoutMillis: Long = 5_000, +): Boolean { + if (!isAlive()) return true + + destroy() + return awaitExit(gracePeriodMillis) || run { + killTree() + awaitExit(killTimeoutMillis) + } +} + +private suspend fun Process.awaitExit(timeoutMillis: Long): Boolean { + var waited = 0L + while (waited < timeoutMillis) { + if (!isAlive()) return true + delay(EXIT_POLL_INTERVAL_MILLIS) + waited += EXIT_POLL_INTERVAL_MILLIS + } + return !isAlive() +} + +private const val EXIT_POLL_INTERVAL_MILLIS = 100L