From efc17c4e500fd36eb72c8d93eb2278df0db823cb Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 13 Sep 2026 18:15:17 +0000 Subject: [PATCH] tstest/integration: fix ETXTBSY race when copying test binaries Tests that exec a fresh CopyTo copy of the tailscale/tailscaled binaries occasionally failed with "text file busy" on GitHub Actions, previously worked around with the retry loop in awaitTailscaledRunnable. The root cause: CopyTo's Linux hardlink fast path only works while the built binary is still linked somewhere. The kernel refuses to hardlink an inode whose link count is zero, even via the still-open FD, so once the building test's TempDir (and every other test's copy) has been cleaned up, later tests silently fall through to the byte-copy path. That path writes the new copy from the test process itself, and if another parallel test forks a child while the write FD is open, the child holds the inherited FD (O_CLOEXEC only closes at exec, not at fork) and a subsequent exec of the fresh copy fails with ETXTBSY. This is golang.org/issue/22315. On macOS and the BSDs, CopyTo always takes the byte-copy path, so every copy races there. Fix it the way the syscall package documents: hold ForkLock for reading across the copy so that no fork overlaps the lifetime of the write FD. A standalone stress program reproduced the race in 86 of 200 execs under a fork storm and in 0 of 200 with the lock held. Updates #15868 Updates #15865 Change-Id: I8a6be72826c9073a9187d7e6c90c8733d2dadb05 Signed-off-by: Brad Fitzpatrick --- tstest/integration/integration.go | 65 ++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 22 deletions(-) diff --git a/tstest/integration/integration.go b/tstest/integration/integration.go index 162bbb0ee..ffebe7717 100644 --- a/tstest/integration/integration.go +++ b/tstest/integration/integration.go @@ -31,6 +31,7 @@ "strconv" "strings" "sync" + "syscall" "testing" "time" @@ -113,29 +114,16 @@ func (b BinaryInfo) CopyTo(dir string) (BinaryInfo, error) { // full copy of the binary. We can't use os.Link(b.Path, ret.Path) // because b.Path is in the first test's TempDir, which may be // cleaned up before later tests call CopyTo. The open FD keeps the - // inode alive after the path is deleted. + // inode alive after the path is deleted, but only for reading: + // once the inode's link count drops to zero the kernel refuses + // to hardlink it again, so this fails and we fall through to + // copying the bytes instead. if err := tryLinkat(b.FD, ret.Path); err == nil { return ret, nil } fallthrough case "darwin", "freebsd", "openbsd", "netbsd": - f, err := os.OpenFile(ret.Path, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o755) - if err != nil { - return BinaryInfo{}, err - } - b.FDMu.Lock() - b.FD.Seek(0, 0) - size, err := io.Copy(f, b.FD) - b.FDMu.Unlock() - if err != nil { - f.Close() - return BinaryInfo{}, fmt.Errorf("copying %q: %w", b.Path, err) - } - if size != b.Size { - f.Close() - return BinaryInfo{}, fmt.Errorf("copy %q: size mismatch: %d != %d", b.Path, size, b.Size) - } - if err := f.Close(); err != nil { + if err := b.writeCopy(ret.Path); err != nil { return BinaryInfo{}, err } return ret, nil @@ -146,6 +134,38 @@ func (b BinaryInfo) CopyTo(dir string) (BinaryInfo, error) { } } +// writeCopy writes the binary's contents from b.FD to path. +// +// It holds syscall.ForkLock for reading for the duration of the write +// so that no concurrently forked child inherits the transient write +// FD. A forked child holds inherited FDs (even O_CLOEXEC ones) until +// it execs, and an exec of the new copy fails with ETXTBSY as long as +// any process holds a write FD on it (golang.org/issue/22315). This +// was the cause of the once-mysterious ETXTBSY errors +// (https://github.com/tailscale/tailscale/issues/15868) that +// [TestNode.awaitTailscaledRunnable] retries around. +func (b BinaryInfo) writeCopy(path string) error { + syscall.ForkLock.RLock() + defer syscall.ForkLock.RUnlock() + + f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o755) + if err != nil { + return err + } + defer f.Close() + b.FDMu.Lock() + b.FD.Seek(0, 0) + size, err := io.Copy(f, b.FD) + b.FDMu.Unlock() + if err != nil { + return fmt.Errorf("copying %q: %w", b.Path, err) + } + if size != b.Size { + return fmt.Errorf("copy %q: size mismatch: %d != %d", b.Path, size, b.Size) + } + return f.Close() +} + // GetBinaries create a temp directory using tb and builds (or copies previously // built) cmd/tailscale and cmd/tailscaled binaries into that directory. // @@ -890,10 +910,11 @@ type waitResult struct { } // awaitTailscaledRunnable tries to run `tailscaled --version` until it -// works. This is an unsatisfying workaround for ETXTBSY we were seeing -// on GitHub Actions that aren't understood. It's not clear what's holding -// a writable fd to tailscaled after `go install` completes. -// See https://github.com/tailscale/tailscale/issues/15868. +// works. It began as a workaround for mysterious ETXTBSY errors on +// GitHub Actions (https://github.com/tailscale/tailscale/issues/15868), +// whose cause is now understood and fixed (see [BinaryInfo.writeCopy]). +// It remains as cheap insurance against any other transient exec +// failure. func (n *TestNode) awaitTailscaledRunnable() error { t := n.env.t t.Helper()