Skip to content

fix: wait for the browser to actually exit when stopping it - #84

Merged
nathanfallet merged 3 commits into
mainfrom
fix/wait-for-the-browser-to-actually-exit
Aug 27, 2026
Merged

fix: wait for the browser to actually exit when stopping it#84
nathanfallet merged 3 commits into
mainfrom
fix/wait-for-the-browser-to-actually-exit

Conversation

@nathanfallet

Copy link
Copy Markdown
Member

Problem

stop() asked the browser to terminate and moved on:

withTimeoutOrNull(5.seconds) { connection?.browser?.close() }
process?.destroy()
process = null

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.

So a browser started on the same --user-data-dir shortly afterwards 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.

We hit this in production as a restart loop: close the browser, reopen it one second later, fail for 30s, repeat — recoverable only by rebooting the machine, which is simply what finally kills the leftover children.

15:44:05  restart requested → stop() → destroy() (async)
15:44:06  new browser started on the same profile   ← one second later
15:44:53  failed: debug port never opened           ← 31s timeout

Fix

stop() now waits:

process?.let { if (!it.destroyAndAwaitExit()) logger.warn("Browser process ${it.pid()} still alive after kill") }

destroyAndAwaitExit() follows the same shape as zendriver's Browser.stop (terminate → poll up to 3s → kill → wait):

  1. destroy(), then poll isAlive() for a grace period (3s by default);
  2. still alive → killTree();
  3. poll again (5s by default);
  4. return whether it actually exited, so the caller can log if it did not.

The waiting loop is common codeisAlive() exists on every target — so only the forceful kill needed a per-platform actual:

  • JVM: killTree() also takes down descendants. They are snapshot before killing the parent, since killing it detaches them and they can no longer be enumerated.
  • POSIX (linux/macos): SIGKILL to the process itself, which is enough there — unlike Windows, children do not outlive their parent.

Notes

No API change beyond the new Process.killTree() expect/actual and the destroyAndAwaitExit() helper. Timeouts are parameters with sane defaults.

:core:jvmTest passes. :core:build fails on kotlinStoreYarnLock, as on main — unrelated.

@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

@nathanfallet
nathanfallet force-pushed the fix/wait-for-the-browser-to-actually-exit branch from c62bd5b to 22759b7 Compare August 26, 2026 18:19
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.
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.
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.
@nathanfallet
nathanfallet force-pushed the fix/wait-for-the-browser-to-actually-exit branch from 58c99b3 to c7fbfa2 Compare August 27, 2026 08:35
@nathanfallet
nathanfallet merged commit 51acc09 into main Aug 27, 2026
4 of 5 checks passed
@nathanfallet
nathanfallet deleted the fix/wait-for-the-browser-to-actually-exit branch August 27, 2026 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant