From 51a7ea302f7e3b81fe4cbbd7d53f6d7f2d183096 Mon Sep 17 00:00:00 2001 From: Steve Cliff Date: Mon, 4 May 2026 09:43:27 +0100 Subject: [PATCH] test: write-then-rename script-bin helpers (avoid ETXTBSY under -race) CI run #48 failed with: --- FAIL: TestRunInitShipsStartedAndFinished RunInit: ... fork/exec /tmp/.../restic: text file busy setupScript and setupScriptBin used os.WriteFile to write a shell script directly at the final path, then exec'd it. Under -race + many t.Parallel tests, a fork-from-another-goroutine could inherit the still-open writable fd from one of those WriteFile calls; the kernel returns ETXTBSY when the freshly-execed binary still has a writable fd anywhere on the system. Fix: write to ".tmp", then os.Rename into place. The rename is a pure dirent op; by the time the final path exists, no process has a writable fd on its inode and exec is safe. -race + -count=5 on both runner packages now passes consistently. --- internal/agent/runner/runner_test.go | 20 ++++++++++++++++---- internal/restic/runner_test.go | 16 ++++++++++++---- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/internal/agent/runner/runner_test.go b/internal/agent/runner/runner_test.go index a5a817f..c9fb042 100644 --- a/internal/agent/runner/runner_test.go +++ b/internal/agent/runner/runner_test.go @@ -20,14 +20,26 @@ func (s *fakeSender) Send(e api.Envelope) error { // setupScript writes a shell script (without shebang) to a temp dir, // names it "restic", makes it executable, and returns the path. +// +// Writes to ".tmp" then renames into place. The rename is what +// makes this race-free: under -race + many t.Parallel tests, a +// fork-from-another-goroutine can inherit the writable fd from +// os.WriteFile before close completes, and exec'ing the file then +// returns ETXTBSY ("text file busy"). Once the rename lands, the +// final path is a fresh dirent pointing at an inode that has no +// writable fd open anywhere — exec is safe. func setupScript(t *testing.T, body string) string { t.Helper() dir := t.TempDir() - p := filepath.Join(dir, "restic") - if err := os.WriteFile(p, []byte("#!/bin/sh\n"+body+"\n"), 0o755); err != nil { - t.Fatalf("setupScript: %v", err) + final := filepath.Join(dir, "restic") + tmp := final + ".tmp" + if err := os.WriteFile(tmp, []byte("#!/bin/sh\n"+body+"\n"), 0o755); err != nil { + t.Fatalf("setupScript: write tmp: %v", err) } - return p + if err := os.Rename(tmp, final); err != nil { + t.Fatalf("setupScript: rename: %v", err) + } + return final } // firstEnvOfType returns the first envelope with the given type, or diff --git a/internal/restic/runner_test.go b/internal/restic/runner_test.go index de24ef2..a2d6708 100644 --- a/internal/restic/runner_test.go +++ b/internal/restic/runner_test.go @@ -13,15 +13,23 @@ import ( // makes it executable, and returns its path. scriptBody is the // complete script content (without the shebang line — that's added // automatically). +// Writes to ".tmp" then renames into place — see the matching +// helper in internal/agent/runner/runner_test.go for the ETXTBSY +// race rationale. Same fix applied here so this helper doesn't lose +// the race the next time CI gets unlucky. func setupScriptBin(t *testing.T, scriptBody string) string { t.Helper() dir := t.TempDir() - p := filepath.Join(dir, "restic") + final := filepath.Join(dir, "restic") + tmp := final + ".tmp" content := "#!/bin/sh\n" + scriptBody + "\n" - if err := os.WriteFile(p, []byte(content), 0o755); err != nil { - t.Fatalf("setupScriptBin: %v", err) + if err := os.WriteFile(tmp, []byte(content), 0o755); err != nil { + t.Fatalf("setupScriptBin: write tmp: %v", err) } - return p + if err := os.Rename(tmp, final); err != nil { + t.Fatalf("setupScriptBin: rename: %v", err) + } + return final } // captureLines returns a LineHandler that appends "stream:line" into