fix: stop self-upgrade tests from fork-bombing the windows test runner

The test:windows CI job intermittently failed to clean up .testbin with
"Access to the path '...cmd_cli.test.exe' is denied". This was previously
attributed to Windows Defender scanning the large unsigned test binaries,
and mitigated with Defender exclusions and cleanup retries. That was
treating a symptom.

Root cause: performUpgrade() self-upgrades by running
exec.Command(os.Executable(), "upgrade", "prod", "-vv") as a detached,
windowless child. In the real ctrld binary this re-execs ctrld and is
correct. Under `go test`, os.Executable() is the test binary itself, and
`go test` stops flag parsing at the first positional arg ("upgrade") and
ignores the rest -- so the child silently re-runs the entire test suite.
That child hits the upgrade tests again and spawns more detached children,
recursively: a fork bomb of hidden processes that pins the runner's
CPU/memory and keeps the test binary's image file locked. Windows refuses
to delete the image of a running process, hence the "Access is denied"
during after_script. Whether any children are still alive when cleanup
runs is a timing race, which is why the failure was flaky.

Two tests reached this path: Test_performUpgrade (directly) and
Test_selfUpgradeCheck (via selfUpgradeCheck -> performUpgrade on the
"upgrade allowed" case).

Fix:
- prog.go: extract the command construction into a package-level
  newUpgradeCmd var. Production behavior is unchanged.
- main_test.go: stub newUpgradeCmd once in TestMain so the whole test
  binary self-execs with `-test.run=^$` (matches no tests, exits
  immediately) instead of re-running the suite. This covers every test
  that reaches performUpgrade, present and future, while still exercising
  the cmd.Start() success path.
This commit is contained in:
Cuong Manh Le
2026-06-15 15:50:12 +07:00
committed by Cuong Manh Le
parent 1e1c998c89
commit 723c7827ba
3 changed files with 32 additions and 2 deletions
+16
View File
@@ -2,6 +2,7 @@ package cli
import (
"os"
"os/exec"
"strings"
"testing"
@@ -13,5 +14,20 @@ var logOutput strings.Builder
func TestMain(m *testing.M) {
l := zerolog.New(&logOutput)
mainLog.Store(&l)
// Stub the self-upgrade command builder for the whole test binary. The real
// builder execs os.Executable() — which under `go test` IS this test binary
// — with positional args ("upgrade", ...). `go test` stops flag parsing at
// the first positional arg and ignores the rest, so the child just re-runs
// the entire suite, hits the upgrade tests again, and spawns more children:
// a fork bomb of detached processes that stalls the host and (on Windows)
// holds the test binary's image locked, breaking CI artifact cleanup.
// Point it at the test binary with a no-match -test.run so any test that
// reaches performUpgrade still exercises the cmd.Start() success path while
// the child exits immediately without recursing.
newUpgradeCmd = func(exe string) *exec.Cmd {
return exec.Command(exe, "-test.run=^$")
}
os.Exit(m.Run())
}
+14 -2
View File
@@ -1651,6 +1651,19 @@ func shouldUpgrade(vt string, cv *semver.Version, logger *zerolog.Logger) bool {
return true
}
// newUpgradeCmd builds the detached command used to self-upgrade. It is a
// package-level variable so tests can stub it. With the real implementation a
// *test* binary would re-exec itself — os.Executable() is the test binary, and
// because `go test` stops flag parsing at the first positional arg ("upgrade")
// it ignores the args and re-runs the entire suite. That child hits the same
// upgrade test and spawns another child, recursively: a fork bomb of detached
// processes that pins the host and locks the test binary's image file.
var newUpgradeCmd = func(exe string) *exec.Cmd {
cmd := exec.Command(exe, "upgrade", "prod", "-vv")
cmd.SysProcAttr = sysProcAttrForDetachedChildProcess()
return cmd
}
// performUpgrade executes the self-upgrade command.
// Returns true if upgrade was initiated successfully, false otherwise.
func performUpgrade(vt string) bool {
@@ -1659,8 +1672,7 @@ func performUpgrade(vt string) bool {
mainLog.Load().Error().Err(err).Msg("failed to get executable path, skipped self-upgrade")
return false
}
cmd := exec.Command(exe, "upgrade", "prod", "-vv")
cmd.SysProcAttr = sysProcAttrForDetachedChildProcess()
cmd := newUpgradeCmd(exe)
if err := cmd.Start(); err != nil {
mainLog.Load().Error().Err(err).Msg("failed to start self-upgrade")
return false
+2
View File
@@ -262,6 +262,8 @@ func Test_performUpgrade(t *testing.T) {
},
}
// newUpgradeCmd is stubbed in TestMain so performUpgrade does not re-exec
// (and fork-bomb) the test binary; see the comment there.
for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {