Skip to content

Commit d668b2e

Browse files
fix: address Codex review round 2 on #27924
- run.ps1: read stdout async while reading stderr sync, instead of ReadToEnd()-ing both sequentially - a process writing enough to the stream read second to fill its pipe buffer before the first stream closes would deadlock forever, since nothing drains the second stream while blocked on the first. Reproduced locally with a test program that only writes to stderr (15s hang), fixed, reverified (completes immediately, no regression in the 4-test suite). - run.ps1: use -cnotmatch instead of -notmatch for the expected- substring check - -notmatch is case-insensitive by default in PowerShell, making run.ps1 laxer than run.sh's case-sensitive `case` pattern match and able to miss a diagnostics-wording regression that only changes case. Reproduced (differently-cased substring wrongly "matched"), fixed, reverified (mismatched case now fails, exact case still passes, suite still 4/4). - run.sh: guard `work=$(mktemp -d)` with `|| exit 1` - an unguarded failure left $work empty, so exe="$work/$name.exe" silently became "/name.exe" (filesystem root). Reproduced via WSL with TMPDIR pointed at a nonexistent directory: the script proceeded, tried to write test binaries to /gc_alloc.exe and /hello.exe, and the crash test even produced a false PASS (wrong-reason nonzero exit) masking the real failure. Fixed, reverified (script now exits 1 immediately instead of limping along with a bogus work dir). All from Codex, #27924 pullrequestreview-4767856781.
1 parent 6bd7afb commit d668b2e

2 files changed

Lines changed: 16 additions & 3 deletions

File tree

thirdparty/tccbin_tests/run.ps1

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,22 @@ foreach ($dir in (Get-TestDirs)) {
6666
# returned, for short-lived processes - both reproduced locally).
6767
# ReadToEnd() blocks until the stream closes (i.e. the process has
6868
# actually finished producing output), so there's no race.
69+
#
70+
# stdout and stderr are read one async + one sync (not both
71+
# ReadToEnd()'d sequentially) to avoid deadlocking: a process that
72+
# writes enough to the stream read second to fill its OS pipe
73+
# buffer before the stream read first closes would block forever,
74+
# since nothing drains the second stream while ReadToEnd() blocks
75+
# on the first - reproduced locally with a test program that only
76+
# writes to stderr.
6977
$psi = [System.Diagnostics.ProcessStartInfo]::new($exe)
7078
$psi.RedirectStandardOutput = $true
7179
$psi.RedirectStandardError = $true
7280
$psi.UseShellExecute = $false
7381
$proc = [System.Diagnostics.Process]::Start($psi)
74-
$stdoutText = $proc.StandardOutput.ReadToEnd()
82+
$stdoutTask = $proc.StandardOutput.ReadToEndAsync()
7583
$stderrText = $proc.StandardError.ReadToEnd()
84+
$stdoutText = $stdoutTask.GetAwaiter().GetResult()
7685
$proc.WaitForExit()
7786
$code = $proc.ExitCode
7887
$out = $stdoutText + $stderrText
@@ -84,7 +93,11 @@ foreach ($dir in (Get-TestDirs)) {
8493
elseif ($code -ne [int]$expectExit) {
8594
$ok = $false
8695
}
87-
if ($expectSubstr -ne "" -and $out -notmatch [regex]::Escape($expectSubstr)) {
96+
# -notmatch is case-INSENSITIVE by default in PowerShell, which
97+
# would make this laxer than run.sh's case-sensitive `case`
98+
# pattern match and could miss a diagnostic-wording regression
99+
# that only changes case. -cnotmatch forces case-sensitivity.
100+
if ($expectSubstr -ne "" -and $out -cnotmatch [regex]::Escape($expectSubstr)) {
88101
$ok = $false
89102
}
90103

thirdparty/tccbin_tests/run.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ if [ -n "$platform" ] && [ -d "$here/platform/$platform" ]; then
3434
dirs+=("$here/platform/$platform")
3535
fi
3636

37-
work=$(mktemp -d)
37+
work=$(mktemp -d) || exit 1
3838
trap 'rm -rf "$work"' EXIT
3939

4040
passed=0

0 commit comments

Comments
 (0)