From 5690dcf437f01659a00088e29771b009b70be210 Mon Sep 17 00:00:00 2001 From: Roman Borodavkin Date: Fri, 14 Aug 2026 21:09:13 +0300 Subject: [PATCH 1/3] fix(dictionary): terminate generation process trees - Stop dictionary command descendants after timeout or interruption - Fall back to ProcessHandle teardown when the platform tree kill reports failure - Cover timeout, interruption, and fallback cleanup with real process trees Impact: Failed dictionary generation no longer leaves orphaned subprocesses --- .../files/SdefDictionaryFileGenerator.kt | 35 +++++- .../test/service/DictionaryProcessTest.kt | 116 ++++++++++++++++++ 2 files changed, 146 insertions(+), 5 deletions(-) create mode 100644 src/test/kotlin/com/intellij/plugin/applescript/test/service/DictionaryProcessTest.kt diff --git a/src/main/kotlin/com/intellij/plugin/applescript/lang/dictionary/files/SdefDictionaryFileGenerator.kt b/src/main/kotlin/com/intellij/plugin/applescript/lang/dictionary/files/SdefDictionaryFileGenerator.kt index 0cef4b8..6c80f8a 100644 --- a/src/main/kotlin/com/intellij/plugin/applescript/lang/dictionary/files/SdefDictionaryFileGenerator.kt +++ b/src/main/kotlin/com/intellij/plugin/applescript/lang/dictionary/files/SdefDictionaryFileGenerator.kt @@ -1,5 +1,6 @@ package com.intellij.plugin.applescript.lang.dictionary.files +import com.intellij.execution.process.OSProcessUtil import com.intellij.openapi.components.service import com.intellij.openapi.diagnostic.Logger import com.intellij.openapi.util.SystemInfo @@ -103,11 +104,8 @@ internal object SdefDictionaryFileGenerator { val shellCommand = arrayOf("/bin/bash", "-c", " $cmdName \"$appFilePath\" > $serializePath") LOG.debug("executing command: ${shellCommand.contentToString()}") val execStart = System.currentTimeMillis() - val isFinished = - Runtime - .getRuntime() - .exec(shellCommand) - .waitFor(DICTIONARY_GENERATION_TIMEOUT_SECONDS, TimeUnit.SECONDS) + val process = Runtime.getRuntime().exec(shellCommand) + val isFinished = waitForProcess(process, DICTIONARY_GENERATION_TIMEOUT_SECONDS, TimeUnit.SECONDS) val execEnd = System.currentTimeMillis() if (!isFinished) { if (service().isXcodeInstalled()) { @@ -138,3 +136,30 @@ private data class DictionaryGenerationRequest( val serializePath: String, val isDictionaryFile: Boolean, ) + +internal fun waitForProcess( + process: Process, + timeout: Long, + timeUnit: TimeUnit, + killTree: (Process) -> Boolean = OSProcessUtil::killProcessTree, +): Boolean = + try { + process.waitFor(timeout, timeUnit).also { isFinished -> + if (!isFinished) terminateProcessTree(process, killTree) + } + } catch (interruption: InterruptedException) { + terminateProcessTree(process, killTree) + throw interruption + } + +private fun terminateProcessTree( + process: Process, + killTree: (Process) -> Boolean, +) { + if (killTree(process)) return + + process.descendants().use { descendants -> + descendants.toList().asReversed().forEach(ProcessHandle::destroyForcibly) + } + process.destroyForcibly() +} diff --git a/src/test/kotlin/com/intellij/plugin/applescript/test/service/DictionaryProcessTest.kt b/src/test/kotlin/com/intellij/plugin/applescript/test/service/DictionaryProcessTest.kt new file mode 100644 index 0000000..351db4b --- /dev/null +++ b/src/test/kotlin/com/intellij/plugin/applescript/test/service/DictionaryProcessTest.kt @@ -0,0 +1,116 @@ +package com.intellij.plugin.applescript.test.service + +import com.intellij.openapi.util.SystemInfo +import com.intellij.plugin.applescript.lang.dictionary.files.waitForProcess +import junit.framework.TestCase +import java.io.File +import java.nio.file.Files +import java.util.concurrent.TimeUnit + +class DictionaryProcessTest : TestCase() { + fun testTimeoutKillsTree() { + if (!SystemInfo.isUnix) return + + val processTree = startProcessTree() + try { + assertFalse( + "Timed-out dictionary command must report incomplete execution", + waitForProcess(processTree.parent, 100, TimeUnit.MILLISECONDS), + ) + assertTreeStopped(processTree) + } finally { + processTree.stop() + } + } + + fun testInterruptKillsTree() { + if (!SystemInfo.isUnix) return + + val processTree = startProcessTree() + try { + Thread.currentThread().interrupt() + val thrown = + runCatching { + waitForProcess(processTree.parent, 30, TimeUnit.SECONDS) + }.exceptionOrNull() + + assertTrue("Interrupted dictionary command must propagate interruption", thrown is InterruptedException) + assertTreeStopped(processTree) + } finally { + Thread.interrupted() + processTree.stop() + } + } + + fun testFallbackKillsTree() { + if (!SystemInfo.isUnix) return + + val processTree = startProcessTree() + try { + assertFalse( + "Fallback must preserve the timed-out result", + waitForProcess(processTree.parent, 100, TimeUnit.MILLISECONDS) { false }, + ) + assertTreeStopped(processTree) + } finally { + processTree.stop() + } + } + + private fun startProcessTree(): TestProcessTree { + val childPidFile = Files.createTempFile("dictionary-child-", ".pid").toFile() + val parent = + ProcessBuilder( + "/bin/bash", + "-c", + $$"""sleep 30 & child=$!; printf '%s' "$child" > "$1"; wait""", + "dictionary-process-test", + childPidFile.path, + ).start() + + try { + val deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5) + while (childPidFile.length() == 0L && System.nanoTime() < deadline) { + Thread.sleep(10) + } + assertTrue("Child process PID must be published", childPidFile.length() > 0L) + + val childPid = childPidFile.readText().trim().toLong() + val child = ProcessHandle.of(childPid).orElseThrow() + return TestProcessTree(parent, child, childPidFile) + } catch (failure: Throwable) { + val cleanupFailure = + runCatching { + parent.descendants().use { descendants -> + descendants.forEach(ProcessHandle::destroyForcibly) + } + parent.destroyForcibly() + }.exceptionOrNull() + if (cleanupFailure != null) failure.addSuppressed(cleanupFailure) + childPidFile.delete() + throw failure + } + } + + private fun assertTreeStopped(processTree: TestProcessTree) { + val deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5) + while ((processTree.parent.isAlive || processTree.child.isAlive) && System.nanoTime() < deadline) { + Thread.sleep(10) + } + + assertFalse("Dictionary command parent must stop", processTree.parent.isAlive) + assertFalse("Dictionary command child must stop", processTree.child.isAlive) + } + + private data class TestProcessTree( + val parent: Process, + val child: ProcessHandle, + val childPidFile: File, + ) { + fun stop() { + child.destroyForcibly() + parent.destroyForcibly() + childPidFile.delete() + } + } +} From fac167c307abfa3364c847ade6a3b1304a0bff02 Mon Sep 17 00:00:00 2001 From: Roman Borodavkin Date: Fri, 14 Aug 2026 21:51:08 +0300 Subject: [PATCH 2/3] fix(dictionary): preserve load failures during teardown - Keep timeout and interruption outcomes when cleanup APIs fail - Check every fallback termination request and log contextual cleanup issues - Strengthen process tests for completion, timing, live interruption, and recursive trees Impact: Dictionary load errors remain actionable without orphaning command processes --- .../files/SdefDictionaryFileGenerator.kt | 74 +++++++++++- .../test/service/DictionaryProcessTest.kt | 111 +++++++++++++++--- 2 files changed, 163 insertions(+), 22 deletions(-) diff --git a/src/main/kotlin/com/intellij/plugin/applescript/lang/dictionary/files/SdefDictionaryFileGenerator.kt b/src/main/kotlin/com/intellij/plugin/applescript/lang/dictionary/files/SdefDictionaryFileGenerator.kt index 6c80f8a..35b3188 100644 --- a/src/main/kotlin/com/intellij/plugin/applescript/lang/dictionary/files/SdefDictionaryFileGenerator.kt +++ b/src/main/kotlin/com/intellij/plugin/applescript/lang/dictionary/files/SdefDictionaryFileGenerator.kt @@ -148,18 +148,82 @@ internal fun waitForProcess( if (!isFinished) terminateProcessTree(process, killTree) } } catch (interruption: InterruptedException) { - terminateProcessTree(process, killTree) + terminateProcessTree(process, killTree).forEach(interruption::addSuppressed) throw interruption } private fun terminateProcessTree( process: Process, killTree: (Process) -> Boolean, +): List { + val issues = mutableListOf() + val platformKill = processAttempt { killTree(process) } + platformKill.exceptionOrNull()?.let { issues += TerminationIssue("platform tree kill failed", it) } + if (platformKill.getOrDefault(false)) return emptyList() + + val descendantLookup = processAttempt { process.descendants().use { it.toList().asReversed() } } + descendantLookup.exceptionOrNull()?.let { issues += TerminationIssue("descendant discovery failed", it) } + val descendants = descendantLookup.getOrDefault(emptyList()) + descendants.mapNotNullTo(issues, ::terminateHandle) + + val parentLookup = processAttempt(process::toHandle) + parentLookup.exceptionOrNull()?.let { issues += TerminationIssue("parent handle lookup failed", it) } + val parent = parentLookup.getOrNull() + if (parent != null) { + terminateHandle(parent)?.let(issues::add) + } + + logTerminationIssues(process, issues) + return issues.mapNotNull(TerminationIssue::cause) +} + +private fun terminateHandle(handle: ProcessHandle): TerminationIssue? { + val termination = + processAttempt { + if (!handle.destroyForcibly() && handle.isAlive) { + TerminationIssue("forced termination was rejected for PID ${handle.pid()}") + } else { + null + } + } + return termination.getOrNull() + ?: termination.exceptionOrNull()?.let { + TerminationIssue("forced termination failed for PID ${handle.pid()}", it) + } +} + +private fun logTerminationIssues( + process: Process, + issues: List, ) { - if (killTree(process)) return + if (issues.isEmpty()) return - process.descendants().use { descendants -> - descendants.toList().asReversed().forEach(ProcessHandle::destroyForcibly) + val processId = processAttempt { process.pid().toString() }.getOrDefault("unknown") + val message = + "Dictionary process-tree termination was incomplete for PID $processId: " + + issues.joinToString { it.message } + val primaryCause = issues.firstNotNullOfOrNull(TerminationIssue::cause) + if (primaryCause == null) { + LOG.warn(message) + } else { + LOG.warn(message, primaryCause) } - process.destroyForcibly() } + +private data class TerminationIssue( + val message: String, + val cause: Throwable? = null, +) + +private inline fun processAttempt(action: () -> T): Result = + try { + Result.success(action()) + } catch (failure: IllegalStateException) { + Result.failure(failure) + } catch (failure: IllegalArgumentException) { + Result.failure(failure) + } catch (failure: UnsupportedOperationException) { + Result.failure(failure) + } catch (failure: SecurityException) { + Result.failure(failure) + } diff --git a/src/test/kotlin/com/intellij/plugin/applescript/test/service/DictionaryProcessTest.kt b/src/test/kotlin/com/intellij/plugin/applescript/test/service/DictionaryProcessTest.kt index 351db4b..9cfdd46 100644 --- a/src/test/kotlin/com/intellij/plugin/applescript/test/service/DictionaryProcessTest.kt +++ b/src/test/kotlin/com/intellij/plugin/applescript/test/service/DictionaryProcessTest.kt @@ -5,18 +5,42 @@ import com.intellij.plugin.applescript.lang.dictionary.files.waitForProcess import junit.framework.TestCase import java.io.File import java.nio.file.Files +import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference class DictionaryProcessTest : TestCase() { + fun testCompletionSucceeds() { + if (!SystemInfo.isUnix) return + + val process = ProcessBuilder("/bin/bash", "-c", "exit 0").start() + var killCalled = false + try { + assertTrue( + "Completed dictionary command must report successful execution", + waitForProcess(process, 5, TimeUnit.SECONDS) { + killCalled = true + false + }, + ) + assertFalse("Completed dictionary command must not be terminated", killCalled) + } finally { + process.destroyForcibly() + } + } + fun testTimeoutKillsTree() { if (!SystemInfo.isUnix) return val processTree = startProcessTree() try { + val startedAt = System.nanoTime() assertFalse( "Timed-out dictionary command must report incomplete execution", waitForProcess(processTree.parent, 100, TimeUnit.MILLISECONDS), ) + val elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startedAt) + assertTrue("Dictionary command timeout took ${elapsedMillis}ms", elapsedMillis < 2_000) assertTreeStopped(processTree) } finally { processTree.stop() @@ -27,29 +51,53 @@ class DictionaryProcessTest : TestCase() { if (!SystemInfo.isUnix) return val processTree = startProcessTree() + val workerStarted = CountDownLatch(1) + val thrown = AtomicReference() + val cleanupFailure = IllegalStateException("Synthetic interrupt cleanup failure") + val worker = + Thread { + workerStarted.countDown() + thrown.set( + runCatching { + waitForProcess(processTree.parent, 30, TimeUnit.SECONDS) { throw cleanupFailure } + }.exceptionOrNull(), + ) + } try { - Thread.currentThread().interrupt() - val thrown = - runCatching { - waitForProcess(processTree.parent, 30, TimeUnit.SECONDS) - }.exceptionOrNull() + worker.start() + assertTrue("Dictionary command worker must start", workerStarted.await(5, TimeUnit.SECONDS)) + awaitBlocked(worker) + worker.interrupt() + worker.join(TimeUnit.SECONDS.toMillis(5)) - assertTrue("Interrupted dictionary command must propagate interruption", thrown is InterruptedException) + assertFalse("Interrupted dictionary command worker must stop", worker.isAlive) + val interruption = thrown.get() + assertTrue( + "Interrupted dictionary command must propagate interruption", + interruption is InterruptedException, + ) + assertSame("Cleanup failure must be suppressed", cleanupFailure, interruption?.suppressed?.single()) assertTreeStopped(processTree) } finally { - Thread.interrupted() + worker.interrupt() + worker.join(TimeUnit.SECONDS.toMillis(5)) processTree.stop() } } - fun testFallbackKillsTree() { + fun testKillFallback() { if (!SystemInfo.isUnix) return + assertKillFallback { false } + assertKillFallback { throw IllegalStateException("Synthetic platform kill failure") } + } + + private fun assertKillFallback(killTree: (Process) -> Boolean) { val processTree = startProcessTree() try { assertFalse( "Fallback must preserve the timed-out result", - waitForProcess(processTree.parent, 100, TimeUnit.MILLISECONDS) { false }, + waitForProcess(processTree.parent, 100, TimeUnit.MILLISECONDS, killTree), ) assertTreeStopped(processTree) } finally { @@ -59,11 +107,14 @@ class DictionaryProcessTest : TestCase() { private fun startProcessTree(): TestProcessTree { val childPidFile = Files.createTempFile("dictionary-child-", ".pid").toFile() + val childScript = + $$"""sleep 30 & grandchild=$!; printf "%s %s" "$BASHPID" "$grandchild" > "$1"; wait""" + val parentScript = """bash -c '$childScript' dictionary-child "$1" & wait""" val parent = ProcessBuilder( "/bin/bash", "-c", - $$"""sleep 30 & child=$!; printf '%s' "$child" > "$1"; wait""", + parentScript, "dictionary-process-test", childPidFile.path, ).start() @@ -75,9 +126,13 @@ class DictionaryProcessTest : TestCase() { } assertTrue("Child process PID must be published", childPidFile.length() > 0L) - val childPid = childPidFile.readText().trim().toLong() - val child = ProcessHandle.of(childPid).orElseThrow() - return TestProcessTree(parent, child, childPidFile) + val descendants = + childPidFile + .readText() + .trim() + .split(' ') + .map { ProcessHandle.of(it.toLong()).orElseThrow() } + return TestProcessTree(parent, descendants, childPidFile) } catch (failure: Throwable) { val cleanupFailure = runCatching { @@ -94,21 +149,43 @@ class DictionaryProcessTest : TestCase() { private fun assertTreeStopped(processTree: TestProcessTree) { val deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5) - while ((processTree.parent.isAlive || processTree.child.isAlive) && System.nanoTime() < deadline) { + while ( + (processTree.parent.isAlive || processTree.descendants.any(ProcessHandle::isAlive)) && + System.nanoTime() < deadline + ) { Thread.sleep(10) } assertFalse("Dictionary command parent must stop", processTree.parent.isAlive) - assertFalse("Dictionary command child must stop", processTree.child.isAlive) + assertTrue( + "Dictionary command descendants must stop", + processTree.descendants.none(ProcessHandle::isAlive), + ) + } + + private fun awaitBlocked(worker: Thread) { + val deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5) + while (worker.isAlive && !isWaiting(worker) && System.nanoTime() < deadline) { + Thread.sleep(10) + } + + assertTrue("Dictionary command worker must block while waiting", worker.isAlive) + assertTrue( + "Dictionary command worker must enter a waiting state; got ${worker.state}", + isWaiting(worker), + ) } + private fun isWaiting(worker: Thread): Boolean = + worker.state == Thread.State.WAITING || worker.state == Thread.State.TIMED_WAITING + private data class TestProcessTree( val parent: Process, - val child: ProcessHandle, + val descendants: List, val childPidFile: File, ) { fun stop() { - child.destroyForcibly() + descendants.asReversed().forEach(ProcessHandle::destroyForcibly) parent.destroyForcibly() childPidFile.delete() } From b0e7e01e3771a035d3b1c996347ab59e4cc241bc Mon Sep 17 00:00:00 2001 From: Roman Borodavkin Date: Fri, 14 Aug 2026 22:16:37 +0300 Subject: [PATCH 3/3] fix(dictionary): snapshot descendants before tree kill - Preserve descendant handles before a fallible platform tree kill can reparent them - Add a real-process regression covering parent-first termination fallback Impact: Dictionary timeouts and interruptions cannot leave reparented child processes running --- .../files/SdefDictionaryFileGenerator.kt | 2 +- .../test/service/DictionaryProcessTest.kt | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/com/intellij/plugin/applescript/lang/dictionary/files/SdefDictionaryFileGenerator.kt b/src/main/kotlin/com/intellij/plugin/applescript/lang/dictionary/files/SdefDictionaryFileGenerator.kt index 35b3188..43b88fd 100644 --- a/src/main/kotlin/com/intellij/plugin/applescript/lang/dictionary/files/SdefDictionaryFileGenerator.kt +++ b/src/main/kotlin/com/intellij/plugin/applescript/lang/dictionary/files/SdefDictionaryFileGenerator.kt @@ -157,11 +157,11 @@ private fun terminateProcessTree( killTree: (Process) -> Boolean, ): List { val issues = mutableListOf() + val descendantLookup = processAttempt { process.descendants().use { it.toList().asReversed() } } val platformKill = processAttempt { killTree(process) } platformKill.exceptionOrNull()?.let { issues += TerminationIssue("platform tree kill failed", it) } if (platformKill.getOrDefault(false)) return emptyList() - val descendantLookup = processAttempt { process.descendants().use { it.toList().asReversed() } } descendantLookup.exceptionOrNull()?.let { issues += TerminationIssue("descendant discovery failed", it) } val descendants = descendantLookup.getOrDefault(emptyList()) descendants.mapNotNullTo(issues, ::terminateHandle) diff --git a/src/test/kotlin/com/intellij/plugin/applescript/test/service/DictionaryProcessTest.kt b/src/test/kotlin/com/intellij/plugin/applescript/test/service/DictionaryProcessTest.kt index 9cfdd46..0665dea 100644 --- a/src/test/kotlin/com/intellij/plugin/applescript/test/service/DictionaryProcessTest.kt +++ b/src/test/kotlin/com/intellij/plugin/applescript/test/service/DictionaryProcessTest.kt @@ -92,6 +92,25 @@ class DictionaryProcessTest : TestCase() { assertKillFallback { throw IllegalStateException("Synthetic platform kill failure") } } + fun testReparentedChildKilled() { + if (!SystemInfo.isUnix) return + + val processTree = startProcessTree() + try { + assertFalse( + "Fallback must preserve the timed-out result", + waitForProcess(processTree.parent, 100, TimeUnit.MILLISECONDS) { process -> + process.destroyForcibly() + assertTrue("Platform kill fixture must stop the parent", process.waitFor(5, TimeUnit.SECONDS)) + false + }, + ) + assertTreeStopped(processTree) + } finally { + processTree.stop() + } + } + private fun assertKillFallback(killTree: (Process) -> Boolean) { val processTree = startProcessTree() try {