From c220d3cdb4a28280c32cad4c03ad5ab42c3f2f66 Mon Sep 17 00:00:00 2001 From: Richard Date: Fri, 25 Sep 2026 20:42:59 +0200 Subject: [PATCH] Sandbox: Beenden sammelt den ganzen Baum ein und wartet nicht ewig auf Rohre (#654) Co-Authored-By: Claude Opus 5.5 --- src/ClawdDotNet.Core/Sandbox/BwrapSandbox.cs | 77 ++++++++++++++++++-- 1 file changed, 71 insertions(+), 6 deletions(-) diff --git a/src/ClawdDotNet.Core/Sandbox/BwrapSandbox.cs b/src/ClawdDotNet.Core/Sandbox/BwrapSandbox.cs index 58639ef..1af64a8 100644 --- a/src/ClawdDotNet.Core/Sandbox/BwrapSandbox.cs +++ b/src/ClawdDotNet.Core/Sandbox/BwrapSandbox.cs @@ -251,7 +251,13 @@ public sealed class BwrapSandbox : ISandboxRunner throw; } - await Task.WhenAll(stdout, stderr); + // After a stop, the streams end with the processes — but a reader must never hang on a + // pipe something still holds. Bounded, then the result is what arrived. + var pumps = Task.WhenAll(stdout, stderr); + if (timedOut || output.Truncated) + await Task.WhenAny(pumps, Task.Delay(TimeSpan.FromSeconds(5), CancellationToken.None)); + else + await pumps; if (timedOut) _logger.LogWarning("Sandbox run of {Program} stopped after {Timeout}", request.Executable, request.Limits.Timeout); @@ -280,16 +286,75 @@ public sealed class BwrapSandbox : ISandboxRunner } } - /// bwrap dies, and with it — die-with-parent, own PID namespace — everything inside. + /// + /// Stops the run: everything below bwrap is collected first, then all of it is killed. + /// + /// Not trusting --die-with-parent. The kernel clears the parent-death + /// signal when a process changes its credentials — and bwrap does exactly that while it + /// builds the user namespace. Seen on the test machine: bwrap's monitor was gone, the + /// sandbox lived on under PID 1 and kept the output pipe open. Killing the sandbox's own + /// PID 1 (the init of its PID namespace) takes every process inside with it. + /// private static void Kill(Process process) { + int root; try { - if (!process.HasExited) - process.Kill(entireProcessTree: true); + root = process.Id; } - catch (InvalidOperationException) { } - catch (System.ComponentModel.Win32Exception) { } + catch (InvalidOperationException) + { + return; + } + + foreach (var pid in Descendants(root).Append(root)) + { + try + { + using var victim = Process.GetProcessById(pid); + victim.Kill(); + } + catch (ArgumentException) { } // already gone + catch (InvalidOperationException) { } + catch (System.ComponentModel.Win32Exception) { } + } + } + + /// All processes below , from /proc — collected before anything is killed. + private static IEnumerable Descendants(int root) + { + var children = new Dictionary>(); + foreach (var dir in Directory.EnumerateDirectories("/proc")) + { + if (!int.TryParse(Path.GetFileName(dir), out var pid)) + continue; + try + { + // /proc//stat: "pid (comm) state ppid …" — comm may hold spaces and parentheses. + var stat = File.ReadAllText(Path.Combine(dir, "stat")); + var fields = stat[(stat.LastIndexOf(')') + 2)..].Split(' '); + if (int.TryParse(fields[1], out var parent)) + { + if (!children.TryGetValue(parent, out var list)) + children[parent] = list = []; + list.Add(pid); + } + } + catch (IOException) { } // it ended meanwhile + catch (UnauthorizedAccessException) { } + } + + var result = new List(); + var queue = new Queue([root]); + while (queue.Count > 0 && result.Count < 10_000) + { + foreach (var child in children.GetValueOrDefault(queue.Dequeue()) ?? []) + { + result.Add(child); + queue.Enqueue(child); + } + } + return result; } private static string Kilobytes(long bytes) => Math.Max(1, bytes / 1024).ToString(CultureInfo.InvariantCulture);