From b9e0a8ecbcad27883779b45ea2e05ecc6d8773e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 18:20:29 +0000 Subject: [PATCH] fix(desktop): kill the whole process tree when a now-playing command times out On Linux /bin/sh is dash, which forks the command instead of exec-ing it. The timeout watchdog killed only the shell, the orphaned child kept stdout open, and runCommand's read waited for it however long it ran, so OsNowPlayingReaderTest.runCommandGivesUpOnAHungCommand failed on the ubuntu desktop CI job (30s instead of ~1s). macOS's sh execs the command, which is why it passed there. Destroy the descendants before the process, on timeout and in the finally block. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017KKAj1seHQLCcVEiMeePEE --- .../desktop/nowPlaying/OsNowPlayingReader.kt | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/nowPlaying/OsNowPlayingReader.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/nowPlaying/OsNowPlayingReader.kt index f8aad95b00..b848cc8bc6 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/nowPlaying/OsNowPlayingReader.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/nowPlaying/OsNowPlayingReader.kt @@ -71,7 +71,7 @@ internal suspend fun runCommand( val watchdog = thread(isDaemon = true, name = "now-playing-command-timeout") { try { - if (!started.waitFor(timeoutSeconds, TimeUnit.SECONDS)) started.destroyForcibly() + if (!started.waitFor(timeoutSeconds, TimeUnit.SECONDS)) started.destroyTree() } catch (_: InterruptedException) { // The command finished first. } @@ -87,6 +87,16 @@ internal suspend fun runCommand( Log.d("OsNowPlayingReader") { "${command.first()} failed: ${e.message}" } null } finally { - process?.takeIf { it.isAlive }?.destroyForcibly() + process?.destroyTree() } } + +/** + * Kills [this] and everything it started. Killing only the process is not enough: a shell that forks + * its command (dash, Ubuntu's `/bin/sh`, does) leaves the child holding stdout open, so the read in + * [runCommand] would wait for that child however long it runs, timeout or not. + */ +private fun Process.destroyTree() { + descendants().forEach { it.destroyForcibly() } + if (isAlive) destroyForcibly() +}