fix(shell): interruptible timeout + kill the whole process tree (#62)
withTimeout wrapped a blocking process.waitFor() that coroutine cancellation can't interrupt, so a hung process hung the stage until it exited on its own; and destroyForcibly() killed only the direct child, leaving sh -c grandchildren (the real npm/tsc) as orphans. Use process.waitFor(timeout, MILLISECONDS) for an on-the-clock deadline, and kill via toHandle().descendants() + the parent.
This commit is contained in:
+33
-20
@@ -16,10 +16,9 @@ import com.correx.core.tools.contract.ToolResult
|
|||||||
import com.correx.core.tools.contract.ValidationResult
|
import com.correx.core.tools.contract.ValidationResult
|
||||||
import com.correx.core.tools.process.ChildProcess
|
import com.correx.core.tools.process.ChildProcess
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
import kotlinx.coroutines.TimeoutCancellationException
|
|
||||||
import kotlinx.coroutines.async
|
import kotlinx.coroutines.async
|
||||||
|
import kotlinx.coroutines.coroutineScope
|
||||||
import kotlinx.coroutines.withContext
|
import kotlinx.coroutines.withContext
|
||||||
import kotlinx.coroutines.withTimeout
|
|
||||||
import kotlinx.serialization.json.Json
|
import kotlinx.serialization.json.Json
|
||||||
import kotlinx.serialization.json.JsonObject
|
import kotlinx.serialization.json.JsonObject
|
||||||
import kotlinx.serialization.json.JsonPrimitive
|
import kotlinx.serialization.json.JsonPrimitive
|
||||||
@@ -28,6 +27,7 @@ import kotlinx.serialization.json.buildJsonObject
|
|||||||
import kotlinx.serialization.json.put
|
import kotlinx.serialization.json.put
|
||||||
import kotlinx.serialization.json.putJsonObject
|
import kotlinx.serialization.json.putJsonObject
|
||||||
import java.nio.file.Path
|
import java.nio.file.Path
|
||||||
|
import java.util.concurrent.TimeUnit
|
||||||
|
|
||||||
class ShellTool(
|
class ShellTool(
|
||||||
private val allowedExecutables: Set<String> = emptySet(),
|
private val allowedExecutables: Set<String> = emptySet(),
|
||||||
@@ -195,20 +195,12 @@ class ShellTool(
|
|||||||
runCatching {
|
runCatching {
|
||||||
runCmd(request, process)
|
runCmd(request, process)
|
||||||
}.getOrElse {
|
}.getOrElse {
|
||||||
process.destroyForcibly()
|
killTree(process)
|
||||||
if (it is TimeoutCancellationException) {
|
ToolResult.Failure(
|
||||||
ToolResult.Failure(
|
invocationId = request.invocationId,
|
||||||
invocationId = request.invocationId,
|
reason = it.message ?: "Unknown error occurred during execution",
|
||||||
reason = "Process timed out after ${timeoutMs}ms",
|
recoverable = false,
|
||||||
recoverable = false,
|
)
|
||||||
)
|
|
||||||
} else {
|
|
||||||
ToolResult.Failure(
|
|
||||||
invocationId = request.invocationId,
|
|
||||||
reason = it.message ?: "Unknown error occurred during execution",
|
|
||||||
recoverable = false,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
} ?: ToolResult.Failure(
|
} ?: ToolResult.Failure(
|
||||||
invocationId = request.invocationId,
|
invocationId = request.invocationId,
|
||||||
@@ -254,11 +246,32 @@ class ShellTool(
|
|||||||
.joinToString("\n")
|
.joinToString("\n")
|
||||||
}
|
}
|
||||||
|
|
||||||
private suspend fun runCmd(request: ToolRequest, process: Process): ToolResult = withTimeout(timeoutMs) {
|
// Kill the whole process tree. destroyForcibly() signals only the direct child, so an `sh -c`
|
||||||
val stdoutDeferred = async { process.inputStream.bufferedReader().use { it.readText() } }
|
// command's grandchildren (the actual npm/tsc/…) would survive as orphans; descendants() reaches
|
||||||
val stderrDeferred = async { process.errorStream.bufferedReader().use { it.readText() } }
|
// them. Snapshot descendants BEFORE destroying the parent — reparenting can hide them afterwards.
|
||||||
|
private fun killTree(process: Process) {
|
||||||
|
process.toHandle().descendants().forEach { it.destroyForcibly() }
|
||||||
|
process.destroyForcibly()
|
||||||
|
}
|
||||||
|
|
||||||
val exitCode = process.waitFor()
|
private suspend fun runCmd(request: ToolRequest, process: Process): ToolResult = coroutineScope {
|
||||||
|
val stdoutDeferred = async(Dispatchers.IO) { process.inputStream.bufferedReader().use { it.readText() } }
|
||||||
|
val stderrDeferred = async(Dispatchers.IO) { process.errorStream.bufferedReader().use { it.readText() } }
|
||||||
|
|
||||||
|
// waitFor(timeout) enforces the deadline on the clock — the plain blocking waitFor() is not
|
||||||
|
// coroutine-cancellable, so a withTimeout around it can't actually interrupt a hung process.
|
||||||
|
val finished = withContext(Dispatchers.IO) { process.waitFor(timeoutMs, TimeUnit.MILLISECONDS) }
|
||||||
|
if (!finished) {
|
||||||
|
killTree(process)
|
||||||
|
stdoutDeferred.cancel()
|
||||||
|
stderrDeferred.cancel()
|
||||||
|
return@coroutineScope ToolResult.Failure(
|
||||||
|
invocationId = request.invocationId,
|
||||||
|
reason = "Process timed out after ${timeoutMs}ms",
|
||||||
|
recoverable = false,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
val exitCode = process.exitValue()
|
||||||
val stdout = stdoutDeferred.await()
|
val stdout = stdoutDeferred.await()
|
||||||
val stderr = stderrDeferred.await()
|
val stderr = stderrDeferred.await()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user