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..e211166e2 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,13 @@ 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/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 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/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/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, 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