Deep Review: 20260411-220257-pr-292

    run: |
      $outputDir = "${{ inputs.output-dir }}"
      $pidFile = "$outputDir/typeperf.pid"
      if (Test-Path $pidFile) {
        $typeperfPid = Get-Content $pidFile
        Stop-Process -Id $typeperfPid -Force -ErrorAction SilentlyContinue
        Wait-Process -Id $typeperfPid -Timeout 5 -ErrorAction SilentlyContinue
        Remove-Item $pidFile -Force -ErrorAction SilentlyContinue
        Write-Output "stopped typeperf pid=$typeperfPid"
      }
      $blgFile = "$outputDir/typeperf.blg"
      $tsvFile = "$outputDir/typeperf.tsv"
      if (Test-Path $blgFile) {
        $size = (Get-Item $blgFile).Length / 1MB
        Write-Output ("typeperf.blg: {0:N1} MB" -f $size)
        relog $blgFile -f TSV -o $tsvFile -y
        if ($LASTEXITCODE -ne 0 -or -not (Test-Path $tsvFile)) {
          Write-Warning "relog failed; preserving $blgFile for inspection"
          exit 1
        }

Stop-Process -Force issues TerminateProcess, which is asynchronous: the kernel signals cancellation but the process object and its file handles are not fully released until every outstanding I/O completes. On a busy runner typeperf can still hold typeperf.blg for a few milliseconds when relog starts, producing a file-sharing violation. The race window is tiny and this is defensive rather than a demonstrated failure, but the cost of a lost summary partly defeats the instrumentation, so it is worth eliminating.

Gemini rated this Important; I downgraded it to a Suggestion because typeperf flushes each BLG sample immediately and Windows file-handle teardown after TerminateProcess is usually complete within milliseconds, so the practical failure rate on CI runners is near zero. Wait-Process is still the right call.

Fix:

 Stop-Process -Id $typeperfPid -Force -ErrorAction SilentlyContinue
+Wait-Process -Id $typeperfPid -Timeout 5 -ErrorAction SilentlyContinue
 Remove-Item $pidFile -Force -ErrorAction SilentlyContinue

// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: SUSE LLC
// SPDX-FileCopyrightText: The Rancher Desktop Authors

// summarize-typeperf reads a TSV file produced by typeperf (via relog
// from BLG) and prints a compact summary of system counters and top
// processes per sample.
//
// Usage:
//
//	go run .github/actions/windows-typeperf-sampler/summarize-typeperf.go <perfdata.tsv> [topN]
package main

import (
	"encoding/csv"

The file is summarize-typeperf.go under .github/actions/windows-typeperf-sampler/, and the workflow invokes that exact path from action.yaml:55-56. The header refers to a summarize-perf-counters name and scripts/summarize-perf-counters.go path that git log --all -- scripts/summarize* confirms has never existed in this repo — it looks like leftover boilerplate from an external adaptation. Anyone trying to reproduce the summary locally will be sent to a file that is not there.

The companion fmt.Fprintf(os.Stderr, "usage: summarize-perf-counters ...") at line 46 has the same stale name.

	handles int
	threads int
}

var headerRe = regexp.MustCompile(`\\Process\(([^)]+)\)\\(.+)$`)

func main() {
	if len(os.Args) < 2 {
		fmt.Fprintf(os.Stderr, "usage: summarize-typeperf <perfdata.tsv> [topN]\n")
		os.Exit(1)
	}

Fix:

-// summarize-perf-counters reads a TSV file produced by typeperf and prints
-// a compact summary of system counters and top processes per sample.
+// summarize-typeperf reads a TSV file produced by typeperf (via relog from
+// BLG) and prints a compact summary of system counters and top processes
+// per sample.
 //
 // Usage:
 //
-//	go run scripts/summarize-perf-counters.go <perfdata.tsv> [topN]
+//	go run .github/actions/windows-typeperf-sampler/summarize-typeperf.go <perfdata.tsv> [topN]
 package main

And update the usage line at line 46 to match.

      run: |
        wsl --list --verbose
        wsl --status
      continue-on-error: true

    - name: "Windows: capture machine info"
      if: runner.os == 'Windows'
      uses: ./.github/actions/windows-machine-info
      with:
        output-path: rdd-logs/_diag/machine-info.txt
      continue-on-error: true

    - name: "Windows: start typeperf sampler"
      if: runner.os == 'Windows'
      uses: ./.github/actions/windows-typeperf-sampler
      with:
        command: start
        output-dir: rdd-logs/_diag
      continue-on-error: true

    - run: make -C bats --keep-going --jobs=$(nproc) --output-sync=target
      shell: run-in-shell {0}
      env:
        RDD_TRACE: true
        RDD_LOG_LEVEL: trace

machine-info.ps1:79-128 deliberately writes and reads back a 256 MB sequential file on the same volume as the BATS workload, and the code comment at lines 80-86 explicitly notes that this measures the cache-warm sequential rate (not raw disk) because "FileStream with no buffering still goes through the OS file cache". That is fine as a runner-characterization data point, but placing the step right before the BATS step means every Windows job now starts with a 256 MB warm-up burst that both drains ~5 seconds of wall clock and changes the disk/cache state before the test the diagnostic is meant to correlate with begins.

        '\PhysicalDisk(_Total)\Disk Write Bytes/sec',
        '\PhysicalDisk(_Total)\Disk Read Bytes/sec',
        '\PhysicalDisk(_Total)\Current Disk Queue Length'
    ) -ErrorAction SilentlyContinue | Format-List

    Write-Output "=== Disk Throughput Benchmark ==="
    # Sequential write/read of a 256 MB file in TEMP. This measures the disk
    # path the bats run actually uses (Lima's instance dirs and the lima
    # download cache live under %USERPROFILE% / %LOCALAPPDATA%, on the same
    # volume as %TEMP% on a default GitHub runner). FileStream with no
    # buffering still goes through the OS file cache, so this is the
    # cache-warm sequential rate, not raw disk speed -- but that is what
    # gzip writes hit too.
    $benchFile = Join-Path $env:TEMP "diag-disk-bench-$([System.Guid]::NewGuid().ToString('N')).bin"
    try {
        $sizeMB = 256
        $buf = New-Object byte[] (1MB)
        (New-Object Random).NextBytes($buf)

        $sw = [System.Diagnostics.Stopwatch]::StartNew()
        $fs = [System.IO.File]::Create($benchFile)
        try {
            for ($i = 0; $i -lt $sizeMB; $i++) {
                $fs.Write($buf, 0, $buf.Length)
            }
            $fs.Flush($true)  # flush to disk
        } finally {
            $fs.Dispose()
        }
        $sw.Stop()
        $writeMs = $sw.Elapsed.TotalMilliseconds
        $writeMBs = [math]::Round($sizeMB * 1000 / $writeMs, 1)

        $sw.Restart()
        $fs = [System.IO.File]::OpenRead($benchFile)
        try {
            $total = 0
            $readbuf = New-Object byte[] (1MB)
            while (($n = $fs.Read($readbuf, 0, $readbuf.Length)) -gt 0) {
                $total += $n
            }
        } finally {
            $fs.Dispose()
        }
        $sw.Stop()
        $readMs = $sw.Elapsed.TotalMilliseconds
        $readMBs = [math]::Round($sizeMB * 1000 / $readMs, 1)

        [PSCustomObject]@{
            File = $benchFile
            SizeMB = $sizeMB
            WriteMs = [math]::Round($writeMs, 1)
            WriteMBPerSec = $writeMBs
            ReadMs  = [math]::Round($readMs, 1)
            ReadMBPerSec  = $readMBs
        } | Format-List
    } catch {
        Write-Output "disk bench failed: $($_.Exception.Message)"
    } finally {
        if (Test-Path $benchFile) { Remove-Item $benchFile -Force -ErrorAction SilentlyContinue }

      if: runner.os == 'macOS'
      working-directory: ${{ runner.temp }}
      run: |
        brew install bash coreutils make

        # Set up run-in-shell helper script (as a symlink to bash)
        mkdir -p bin
        ln -s $(brew --prefix bash)/bin/bash bin/run-in-shell
        readlink -f bin >> "$GITHUB_PATH"

        # Use GNU make
        echo "$(brew --prefix)/opt/make/libexec/gnubin" >> "$GITHUB_PATH"

    - name: "Windows: install prerequisites"
      if: runner.os == 'Windows'
      shell: pwsh
      working-directory: ${{ runner.temp }}

Fix: either move the machine-info step to run after the BATS step (runner characteristics do not change during the job), or gate the disk benchmark behind an action input and default it off.

    # download cache live under %USERPROFILE% / %LOCALAPPDATA%, on the same
    # volume as %TEMP% on a default GitHub runner). FileStream with no
    # buffering still goes through the OS file cache, so this is the
    # cache-warm sequential rate, not raw disk speed -- but that is what
    # gzip writes hit too.
    $benchFile = Join-Path $env:TEMP "diag-disk-bench-$([System.Guid]::NewGuid().ToString('N')).bin"
    try {
        $sizeMB = 256
        $buf = New-Object byte[] (1MB)
        (New-Object Random).NextBytes($buf)

        $sw = [System.Diagnostics.Stopwatch]::StartNew()
        $fs = [System.IO.File]::Create($benchFile)
        try {
            for ($i = 0; $i -lt $sizeMB; $i++) {
                $fs.Write($buf, 0, $buf.Length)
            }
            $fs.Flush($true)  # flush to disk
        } finally {
            $fs.Dispose()
        }
        $sw.Stop()
        $writeMs = $sw.Elapsed.TotalMilliseconds
        $writeMBs = [math]::Round($sizeMB * 1000 / $writeMs, 1)

        $sw.Restart()
        $fs = [System.IO.File]::OpenRead($benchFile)
        try {
            $total = 0
            $readbuf = New-Object byte[] (1MB)
            while (($n = $fs.Read($readbuf, 0, $readbuf.Length)) -gt 0) {
                $total += $n
            }
        } finally {
            $fs.Dispose()
        }
        $sw.Stop()
        $readMs = $sw.Elapsed.TotalMilliseconds

If $fs.Write throws mid-loop (disk full, antivirus locking, I/O error) control jumps past $fs.Close() into the outer catch block. The handle is leaked to GC. The finally then tries Remove-Item $benchFile -Force -ErrorAction SilentlyContinue on a still-locked file; with SilentlyContinue the delete failure is swallowed and a 256 MB file lingers under %TEMP% until the runner is recycled. The identical pattern repeats in the read loop at lines 104-111.

Fix: wrap each stream in its own try/finally so the handle is disposed deterministically.

-        $fs = [System.IO.File]::Create($benchFile)
-        for ($i = 0; $i -lt $sizeMB; $i++) {
-            $fs.Write($buf, 0, $buf.Length)
-        }
-        $fs.Flush($true)  # flush to disk
-        $fs.Close()
+        $fs = [System.IO.File]::Create($benchFile)
+        try {
+            for ($i = 0; $i -lt $sizeMB; $i++) {
+                $fs.Write($buf, 0, $buf.Length)
+            }
+            $fs.Flush($true)  # flush to disk
+        } finally {
+            $fs.Dispose()
+        }

	"math"
	"os"
	"regexp"
	"slices"
	"strconv"
	"strings"
)

type procSnap struct {
	name    string
	pid     int
	cpu     float64
	wsMB    float64
	handles int
	threads int
}

// procIndex holds column indices for one process instance.

printTopN only prints name, pid, cpu, wsMB:

fmt.Printf("    %-30s PID %6d  CPU %7.1f%%  WS %8.1f MB\n",
    s.name, s.pid, s.cpu, s.wsMB)

The handles/threads fields on procSnap, the matching fields on procIndex, the header detection for Handle Count/Thread Count, and the per-sample extraction at lines 160-168 all exist only to populate two fields that nothing reads. The corresponding \Process(*)\Handle Count and \Process(*)\Thread Count lines in typeperf-counters.txt each add one column per tracked process per sample to the BLG (and the intermediate TSV) for no benefit.

		}

		// Per-process snapshots via precomputed indices
		snaps := make([]procSnap, 0, len(instances))
		for _, inst := range instances {
			pi := procs[inst]
			cpu, cpuOK := colFloat(record, pi.cpu)
			ws, wsOK := colFloat(record, pi.ws)
			if !cpuOK && !wsOK {
				continue
			}
			pid, _ := colInt(record, pi.pid)
			handles, _ := colInt(record, pi.handles)
			threads, _ := colInt(record, pi.threads)
			snaps = append(snaps, procSnap{
				name:    inst,
				pid:     pid,
				cpu:     cpu,
				wsMB:    ws / (1024 * 1024),

Handle and thread counts are actually useful for leak diagnosis, so extending the output is probably preferable to deleting the counters:

-			fmt.Printf("    %-30s PID %6d  CPU %7.1f%%  WS %8.1f MB\n",
-				s.name, s.pid, s.cpu, s.wsMB)
+			fmt.Printf("    %-30s PID %6d  CPU %7.1f%%  WS %8.1f MB  H %6d  T %4d\n",
+				s.name, s.pid, s.cpu, s.wsMB, s.handles, s.threads)

		case "Thread Count":
			pi.threads = i
		}
	}

	fmt.Printf("Processes tracked: %d\n", len(procs))

	rowNum := 0
	for {
		record, err := reader.Read()
		if errors.Is(err, io.EOF) {
			break
		}
		if err != nil {
			fmt.Fprintf(os.Stderr, "read row %d: %v\n", rowNum+1, err)

encoding/csv.Reader returns io.EOF at clean end, but also returns csv.ErrFieldCount or quoting errors mid-file. The current loop collapses all of them into a silent break, leaving the misleading Samples: N footer at line 176 with a low count and no explanation. Worth distinguishing io.EOF from real parse errors so a partial TSV still summarizes what it can while flagging the failure on stderr.

				pid:     pid,
				cpu:     cpu,
				wsMB:    ws / (1024 * 1024),
				handles: handles,
				threads: threads,
			})
		}

		printTopN(snaps, topN, "CPU", func(s procSnap) float64 { return s.cpu })
		printTopN(snaps, topN, "Working Set", func(s procSnap) float64 { return s.wsMB })
	}

 for {
     record, err := reader.Read()
+    if errors.Is(err, io.EOF) {
+        break
+    }
     if err != nil {
-        break
+        fmt.Fprintf(os.Stderr, "read row %d: %v\n", rowNum+1, err)
+        break
     }
     rowNum++

(Adds errors and io imports.)

      if (-not (Test-Path $outputDir)) {
        New-Item -ItemType Directory -Force -Path $outputDir | Out-Null
      }
      $counterFile = "${{ github.action_path }}/typeperf-counters.txt"
      $blgFile = "$outputDir/typeperf.blg"
      $proc = Start-Process -PassThru -WindowStyle Hidden -FilePath typeperf `
        -ArgumentList "-cf", $counterFile, "-f", "BIN", "-o", $blgFile, "-si", "1", "-y"
      $proc.Id | Set-Content -Path "$outputDir/typeperf.pid"
      Write-Output "typeperf started, pid=$($proc.Id), output=$blgFile"

  - name: Stop typeperf and summarize
    if: runner.os == 'Windows' && inputs.command == 'stop'

If $blgFile already exists, typeperf prompts to overwrite and blocks on stdin. With -WindowStyle Hidden the prompt is invisible and the sampler hangs indefinitely. Fresh CI runners never hit this, but relog on line 51 already passes -y for exactly the same reason; the start step should be symmetric as cheap insurance against a reused runner.

      }
      $blgFile = "$outputDir/typeperf.blg"
      $tsvFile = "$outputDir/typeperf.tsv"
      if (Test-Path $blgFile) {
        $size = (Get-Item $blgFile).Length / 1MB
        Write-Output ("typeperf.blg: {0:N1} MB" -f $size)
        relog $blgFile -f TSV -o $tsvFile -y
        if ($LASTEXITCODE -ne 0 -or -not (Test-Path $tsvFile)) {
          Write-Warning "relog failed; preserving $blgFile for inspection"
          exit 1
        }

-  -ArgumentList "-cf", $counterFile, "-f", "BIN", "-o", $blgFile, "-si", "1"
+  -ArgumentList "-cf", $counterFile, "-f", "BIN", "-o", $blgFile, "-si", "1", "-y"