From 898c7365c48d5d38709545c1df9ee07cf29659dd Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 24 Jul 2026 15:29:21 -0400 Subject: [PATCH] test(sql): close the blackhole gate's leaked-pause race + bound the shell command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on the blackhole gate. Two robustness gaps, both bounded to the disposable dedicated container (never shared infra), fixed: - If the outer token fired while 'docker pause' was in flight, 'paused' was set only AFTER the await, so a cancellation there left paused=false while the container could still complete the pause — the finally then skipped unpause and leaked a frozen container. Mark paused BEFORE the await, and make the finally's unpause best-effort (unpausing a never-actually-paused container errors harmlessly, and a cleanup failure must never mask the real test outcome). - The pause/unpause shell commands had no wall-clock bound of their own — only the post-pause read was capped — so a hung SSH handshake would hang CI. Give RunShellCommandAsync its own 30s hard cap that kills the process and throws TimeoutException, distinct from an operator's own cancellation. Offline skip still clean (1 skipped, 9ms, no socket/docker). Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW --- .../SqlBlackholeTimeoutTests.cs | 41 ++++++++++++++++--- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/SqlBlackholeTimeoutTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/SqlBlackholeTimeoutTests.cs index 3d9baaa4..9cc26756 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/SqlBlackholeTimeoutTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/SqlBlackholeTimeoutTests.cs @@ -125,6 +125,9 @@ public sealed class SqlBlackholeTimeoutTests /// Hard cap on the test's own wait, so a genuinely wedged implementation FAILS rather than hangs CI. private static readonly TimeSpan HardCap = TimeSpan.FromSeconds(60); + /// Hard cap on each docker pause/unpause shell command, so a hung SSH handshake can't hang CI. + private static readonly TimeSpan ShellCommandHardCap = TimeSpan.FromSeconds(30); + /// /// Start reading against the dedicated mssql, docker pause it mid-poll, and assert the next /// read surfaces within (a @@ -178,8 +181,10 @@ public sealed class SqlBlackholeTimeoutTests { // Freeze the container mid-poll: its SQL process is SIGSTOPped, so the pooled connection's next // ExecuteReaderAsync hangs against a live-but-silent socket — the frozen-peer wedge exactly. - await RunShellCommandAsync(DockerCommand("pause", ssh), ct); + // Mark paused BEFORE the await: if `ct` fires mid-command the process may still complete the pause, + // so the finally must attempt an unpause regardless — never leave the container frozen. paused = true; + await RunShellCommandAsync(DockerCommand("pause", ssh), ct); var clock = Stopwatch.StartNew(); // Bounded by HardCap so a wedged implementation FAILS here instead of hanging CI. @@ -212,8 +217,17 @@ public sealed class SqlBlackholeTimeoutTests { if (paused) { - // ALWAYS restore the container once paused — never leave a frozen container behind. - await RunShellCommandAsync(DockerCommand("unpause", ssh), CancellationToken.None); + // ALWAYS restore the container once a pause was attempted — never leave a frozen container + // behind. Best-effort: unpausing a container that never actually paused (pause command failed) + // errors harmlessly, and a cleanup failure must never mask the real test outcome. + try + { + await RunShellCommandAsync(DockerCommand("unpause", ssh), CancellationToken.None); + } + catch + { + // swallow — cleanup is best-effort; the container is disposable and torn down by the runbook. + } } } @@ -388,9 +402,16 @@ public sealed class SqlBlackholeTimeoutTests return string.IsNullOrWhiteSpace(ssh) ? docker : $"ssh {ssh} \"{docker}\""; } - /// Runs a shell one-liner through the login shell, exactly as the S7 blackhole gate does. + /// + /// Runs a shell one-liner through the login shell, exactly as the S7 blackhole gate does — but bounded + /// by its own so a hung SSH handshake (unreachable docker host, an auth + /// prompt) fails the test rather than hanging CI indefinitely. The process is killed on timeout. + /// private static async Task RunShellCommandAsync(string command, CancellationToken ct) { + using var timeout = CancellationTokenSource.CreateLinkedTokenSource(ct); + timeout.CancelAfter(ShellCommandHardCap); + using var proc = new Process { StartInfo = new ProcessStartInfo("/bin/sh", $"-c \"{command.Replace("\"", "\\\"")}\"") @@ -401,7 +422,17 @@ public sealed class SqlBlackholeTimeoutTests }, }; proc.Start(); - await proc.WaitForExitAsync(ct); + try + { + await proc.WaitForExitAsync(timeout.Token); + } + catch (OperationCanceledException) when (timeout.IsCancellationRequested && !ct.IsCancellationRequested) + { + try { proc.Kill(entireProcessTree: true); } catch { /* already gone */ } + throw new TimeoutException( + $"shell command did not complete within {ShellCommandHardCap.TotalSeconds:0}s: {command}"); + } + proc.ExitCode.ShouldBe(0, $"docker command failed: {await proc.StandardError.ReadToEndAsync(ct)}"); // Give the pause/unpause a moment to take effect before the next poll tick. await Task.Delay(TimeSpan.FromSeconds(1), ct);