Skip to content

Commit 4ecbf5e

Browse files
authored
Fix gpk update text file busy on Linux
Patches the rename-aside dance into replace_unix.go so /usr/bin installs work across tmpfs and disk.
2 parents 24e2a9e + f302115 commit 4ecbf5e

2 files changed

Lines changed: 151 additions & 4 deletions

File tree

internal/updater/replace_unix.go

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,44 @@
22

33
package updater
44

5-
// replaceBinary overwrites dest with src. On Unix the running process keeps
6-
// the old inode open via its existing file descriptor, so a straight atomic
7-
// rename is safe even while gpk is still running.
5+
import "os"
6+
7+
// renameFile is os.Rename in production; tests override it to simulate the
8+
// cross-filesystem failure that triggers the rename-aside path.
9+
var renameFile = os.Rename
10+
11+
// replaceBinary overwrites dest with src.
12+
//
13+
// Same-filesystem fast path: a straight rename. The running gpk process
14+
// keeps the old inode open via its existing file descriptor, so the rename
15+
// is safe even while gpk is still running.
16+
//
17+
// Cross-filesystem path: when src lives on a different mount than dest
18+
// (typical: /tmp is tmpfs, dest is /usr/bin on disk), os.Rename returns
19+
// EXDEV. The fallback in moveFile then tries os.OpenFile(dest, O_TRUNC),
20+
// which Linux refuses with ETXTBSY ("text file busy") because dest is the
21+
// currently-running executable.
22+
//
23+
// To get past ETXTBSY we do the same rename-out-of-the-way dance as
24+
// Windows: move dest to "<dest>.old" first (which works for a running
25+
// binary because rename keeps the inode), then copy src into the now-vacant
26+
// dest path. The .old binary can be unlinked immediately afterwards on
27+
// Unix; the kernel preserves the inode for the running process until exit.
828
func replaceBinary(src, dest string) error {
9-
return moveFile(src, dest)
29+
if err := renameFile(src, dest); err == nil {
30+
return nil
31+
}
32+
33+
oldPath := dest + ".old"
34+
_ = os.Remove(oldPath) // best-effort cleanup of a stale .old from a prior failed update
35+
36+
if err := os.Rename(dest, oldPath); err != nil {
37+
return err
38+
}
39+
if err := moveFile(src, dest); err != nil {
40+
_ = os.Rename(oldPath, dest) // roll back so the user isn't left with no binary at dest
41+
return err
42+
}
43+
_ = os.Remove(oldPath) // safe: Unix lets you unlink a running binary; the inode lives on
44+
return nil
1045
}
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
//go:build !windows
2+
3+
package updater
4+
5+
import (
6+
"errors"
7+
"io/fs"
8+
"os"
9+
"path/filepath"
10+
"testing"
11+
)
12+
13+
func TestReplaceBinary_SameFilesystemFastPath(t *testing.T) {
14+
dir := t.TempDir()
15+
src := filepath.Join(dir, "src")
16+
dest := filepath.Join(dir, "dest")
17+
if err := os.WriteFile(src, []byte("new"), 0o755); err != nil {
18+
t.Fatalf("write src: %v", err)
19+
}
20+
if err := os.WriteFile(dest, []byte("old"), 0o755); err != nil {
21+
t.Fatalf("write dest: %v", err)
22+
}
23+
24+
if err := replaceBinary(src, dest); err != nil {
25+
t.Fatalf("replaceBinary: %v", err)
26+
}
27+
28+
body, err := os.ReadFile(dest)
29+
if err != nil {
30+
t.Fatalf("read dest: %v", err)
31+
}
32+
if string(body) != "new" {
33+
t.Errorf("dest = %q, want %q", body, "new")
34+
}
35+
if _, err := os.Stat(src); !errors.Is(err, fs.ErrNotExist) {
36+
t.Errorf("src should be gone after rename, got err=%v", err)
37+
}
38+
}
39+
40+
func TestReplaceBinary_CrossFilesystemUsesRenameAside(t *testing.T) {
41+
dir := t.TempDir()
42+
src := filepath.Join(dir, "src")
43+
dest := filepath.Join(dir, "dest")
44+
if err := os.WriteFile(src, []byte("new"), 0o755); err != nil {
45+
t.Fatalf("write src: %v", err)
46+
}
47+
if err := os.WriteFile(dest, []byte("old"), 0o755); err != nil {
48+
t.Fatalf("write dest: %v", err)
49+
}
50+
51+
// Force the initial rename to fail, mimicking EXDEV when src and dest
52+
// are on different filesystems. The aside path should still succeed
53+
// because moveFile's internal copy creates a fresh dest after the .old
54+
// rename has freed the path.
55+
saved := renameFile
56+
renameFile = func(s, d string) error {
57+
if s == src && d == dest {
58+
return errors.New("simulated cross-filesystem failure")
59+
}
60+
return os.Rename(s, d)
61+
}
62+
t.Cleanup(func() { renameFile = saved })
63+
64+
if err := replaceBinary(src, dest); err != nil {
65+
t.Fatalf("replaceBinary: %v", err)
66+
}
67+
68+
body, err := os.ReadFile(dest)
69+
if err != nil {
70+
t.Fatalf("read dest: %v", err)
71+
}
72+
if string(body) != "new" {
73+
t.Errorf("dest = %q, want %q", body, "new")
74+
}
75+
if _, err := os.Stat(dest + ".old"); !errors.Is(err, fs.ErrNotExist) {
76+
t.Errorf("dest.old should be removed after success, got err=%v", err)
77+
}
78+
}
79+
80+
func TestReplaceBinary_CrossFilesystemRollsBackOnCopyFailure(t *testing.T) {
81+
dir := t.TempDir()
82+
src := filepath.Join(dir, "src")
83+
dest := filepath.Join(dir, "dest")
84+
if err := os.WriteFile(dest, []byte("old"), 0o755); err != nil {
85+
t.Fatalf("write dest: %v", err)
86+
}
87+
// src deliberately does not exist; moveFile inside replaceBinary will
88+
// fail trying to open it for the copy fallback, and the rollback should
89+
// restore dest from the .old we renamed aside.
90+
91+
saved := renameFile
92+
renameFile = func(s, d string) error {
93+
if s == src && d == dest {
94+
return errors.New("simulated cross-filesystem failure")
95+
}
96+
return os.Rename(s, d)
97+
}
98+
t.Cleanup(func() { renameFile = saved })
99+
100+
err := replaceBinary(src, dest)
101+
if err == nil {
102+
t.Fatal("expected an error when src is missing")
103+
}
104+
105+
body, readErr := os.ReadFile(dest)
106+
if readErr != nil {
107+
t.Fatalf("dest should have been rolled back to its original content, but read failed: %v", readErr)
108+
}
109+
if string(body) != "old" {
110+
t.Errorf("dest should still contain old content after rollback, got %q", body)
111+
}
112+
}

0 commit comments

Comments
 (0)