fix(executor): hashFiles skips FIFOs instead of hanging on them
Opening a FIFO for reading blocks until a writer comes, and the context cannot interrupt the open, so a FIFO in the workspace hung the job until its timeout. Files are now opened with O_NONBLOCK; whatever is not a regular file after the open is skipped (the official script would hang on a FIFO). Tested directly, through a symbolic link, with and without --follow-symbolic-links.
- commit
- c9a800a16b3862a9bc6f496799d49a048ccd76f5
- author
- brice · · 2026-10-13 04:04 UTC
- parent
- 7d09faf
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +57 -3
M internal/executor/hashfiles.go +8 -3
| @@ -15,6 +15,7 @@ import ( | |||
| 15 | 15 | "regexp" | |
| 16 | 16 | "sort" | |
| 17 | 17 | "strings" | |
| 18 | + | "syscall" | |
| 18 | 19 | "time" | |
| 19 | 20 | ||
| 20 | 21 | "bnema.dev/ermite-actions/expr" | |
| @@ -717,10 +718,14 @@ func readDirNames(root *os.Root, rel string) ([]string, error) { | |||
| 717 | 718 | } | |
| 718 | 719 | ||
| 719 | 720 | // hashOne is the SHA-256 digest of a regular file. ok is false for what is not | |
| 720 | - | // hashed: a directory (reached through a link), a special file, a link that | |
| 721 | - | // leaves the workspace or is broken. | |
| 721 | + | // hashed: a directory (reached through a link), a special file (a FIFO, a | |
| 722 | + | // socket, a device: skipped, where the official script would hang on a FIFO), | |
| 723 | + | // a link that leaves the workspace or is broken. | |
| 722 | 724 | func hashOne(ctx context.Context, root *os.Root, rel string) (sum []byte, ok bool, err error) { | |
| 723 | - | f, err := root.Open(rel) | |
| 725 | + | // O_NONBLOCK: opening a FIFO for reading blocks until a writer comes, and | |
| 726 | + | // nothing can interrupt the open. Reached through a symbolic link (or with | |
| 727 | + | // --follow-symbolic-links) a FIFO would hang the job until its timeout. | |
| 728 | + | f, err := root.OpenFile(rel, os.O_RDONLY|syscall.O_NONBLOCK, 0) | |
| 724 | 729 | if err != nil { | |
| 725 | 730 | return nil, false, nil | |
| 726 | 731 | } | |
| @@ -15,6 +15,7 @@ import ( | |||||
| 15 | "regexp" | 15 | "regexp" | ||
| 16 | "sort" | 16 | "sort" | ||
| 17 | "strings" | 17 | "strings" | ||
| 18 | + | "syscall" | |||
| 18 | "time" | 19 | "time" | ||
| 19 | 20 | ||||
| 20 | "bnema.dev/ermite-actions/expr" | 21 | "bnema.dev/ermite-actions/expr" | ||
| @@ -717,10 +718,14 @@ func readDirNames(root *os.Root, rel string) ([]string, error) { | |||||
| 717 | } | 718 | } | ||
| 718 | 719 | ||||
| 719 | // hashOne is the SHA-256 digest of a regular file. ok is false for what is not | 720 | // hashOne is the SHA-256 digest of a regular file. ok is false for what is not | ||
| 720 | - | // hashed: a directory (reached through a link), a special file, a link that | 721 | + | // hashed: a directory (reached through a link), a special file (a FIFO, a |
| 721 | - | // leaves the workspace or is broken. | 722 | + | // socket, a device: skipped, where the official script would hang on a FIFO), |
| 723 | + | // a link that leaves the workspace or is broken. | |||
| 722 | func hashOne(ctx context.Context, root *os.Root, rel string) (sum []byte, ok bool, err error) { | 724 | func hashOne(ctx context.Context, root *os.Root, rel string) (sum []byte, ok bool, err error) { | ||
| 723 | - | f, err := root.Open(rel) | 725 | + | // O_NONBLOCK: opening a FIFO for reading blocks until a writer comes, and |
| 726 | + | // nothing can interrupt the open. Reached through a symbolic link (or with | |||
| 727 | + | // --follow-symbolic-links) a FIFO would hang the job until its timeout. | |||
| 728 | + | f, err := root.OpenFile(rel, os.O_RDONLY|syscall.O_NONBLOCK, 0) | |||
| 724 | if err != nil { | 729 | if err != nil { | ||
| 725 | return nil, false, nil | 730 | return nil, false, nil | ||
| 726 | } | 731 | } | ||
M internal/executor/hashfiles_test.go +49 -0
| @@ -7,7 +7,9 @@ import ( | |||
| 7 | 7 | "os/exec" | |
| 8 | 8 | "path/filepath" | |
| 9 | 9 | "strings" | |
| 10 | + | "syscall" | |
| 10 | 11 | "testing" | |
| 12 | + | "time" | |
| 11 | 13 | ) | |
| 12 | 14 | ||
| 13 | 15 | func writeTree(t *testing.T, root string, files map[string]string) { | |
| @@ -139,6 +141,53 @@ func TestHashFilesBrokenLinkFails(t *testing.T) { | |||
| 139 | 141 | } | |
| 140 | 142 | } | |
| 141 | 143 | ||
| 144 | + | // A FIFO in the workspace must not hang the function: opening it for reading | |
| 145 | + | // blocks until a writer comes. It is skipped, directly or through a symbolic | |
| 146 | + | // link, with or without --follow-symbolic-links. | |
| 147 | + | func TestHashFilesSkipsFIFOs(t *testing.T) { | |
| 148 | + | ws := t.TempDir() | |
| 149 | + | writeTree(t, ws, map[string]string{"a.txt": "alpha\n"}) | |
| 150 | + | if err := syscall.Mkfifo(filepath.Join(ws, "pipe"), 0o644); err != nil { | |
| 151 | + | t.Skipf("no FIFO here: %v", err) | |
| 152 | + | } | |
| 153 | + | if err := os.Symlink("pipe", filepath.Join(ws, "pipelink")); err != nil { | |
| 154 | + | t.Fatal(err) | |
| 155 | + | } | |
| 156 | + | only, err := hashFilesIn(t.Context(), ws, "/h", []string{"a.txt"}, false) | |
| 157 | + | if err != nil || only == "" { | |
| 158 | + | t.Fatalf("%q %v", only, err) | |
| 159 | + | } | |
| 160 | + | for _, tc := range []struct { | |
| 161 | + | patterns []string | |
| 162 | + | follow bool | |
| 163 | + | }{ | |
| 164 | + | {[]string{"**"}, false}, {[]string{"**"}, true}, {[]string{"pipe"}, false}, {[]string{"pipe"}, true}, | |
| 165 | + | {[]string{"pipelink"}, false}, {[]string{"pipelink"}, true}, | |
| 166 | + | } { | |
| 167 | + | done := make(chan struct{}) | |
| 168 | + | var got string | |
| 169 | + | var err error | |
| 170 | + | go func() { | |
| 171 | + | defer close(done) | |
| 172 | + | got, err = hashFilesIn(t.Context(), ws, "/h", tc.patterns, tc.follow) | |
| 173 | + | }() | |
| 174 | + | select { | |
| 175 | + | case <-done: | |
| 176 | + | case <-time.After(5 * time.Second): | |
| 177 | + | t.Fatalf("%q follow=%v: hangs on a FIFO", tc.patterns, tc.follow) | |
| 178 | + | } | |
| 179 | + | if err != nil { | |
| 180 | + | t.Errorf("%q follow=%v: %v", tc.patterns, tc.follow, err) | |
| 181 | + | } | |
| 182 | + | if tc.patterns[0] == "**" && got != only { | |
| 183 | + | t.Errorf("%q follow=%v: %q, want a.txt alone %q", tc.patterns, tc.follow, got, only) | |
| 184 | + | } | |
| 185 | + | if tc.patterns[0] != "**" && got != "" { | |
| 186 | + | t.Errorf("%q follow=%v: %q, want no file", tc.patterns, tc.follow, got) | |
| 187 | + | } | |
| 188 | + | } | |
| 189 | + | } | |
| 190 | + | ||
| 142 | 191 | type officialResult struct { | |
| 143 | 192 | sum string | |
| 144 | 193 | failed bool | |
| @@ -7,7 +7,9 @@ import ( | |||||
| 7 | "os/exec" | 7 | "os/exec" | ||
| 8 | "path/filepath" | 8 | "path/filepath" | ||
| 9 | "strings" | 9 | "strings" | ||
| 10 | + | "syscall" | |||
| 10 | "testing" | 11 | "testing" | ||
| 12 | + | "time" | |||
| 11 | ) | 13 | ) | ||
| 12 | 14 | ||||
| 13 | func writeTree(t *testing.T, root string, files map[string]string) { | 15 | func writeTree(t *testing.T, root string, files map[string]string) { | ||
| @@ -139,6 +141,53 @@ func TestHashFilesBrokenLinkFails(t *testing.T) { | |||||
| 139 | } | 141 | } | ||
| 140 | } | 142 | } | ||
| 141 | 143 | ||||
| 144 | + | // A FIFO in the workspace must not hang the function: opening it for reading | |||
| 145 | + | // blocks until a writer comes. It is skipped, directly or through a symbolic | |||
| 146 | + | // link, with or without --follow-symbolic-links. | |||
| 147 | + | func TestHashFilesSkipsFIFOs(t *testing.T) { | |||
| 148 | + | ws := t.TempDir() | |||
| 149 | + | writeTree(t, ws, map[string]string{"a.txt": "alpha\n"}) | |||
| 150 | + | if err := syscall.Mkfifo(filepath.Join(ws, "pipe"), 0o644); err != nil { | |||
| 151 | + | t.Skipf("no FIFO here: %v", err) | |||
| 152 | + | } | |||
| 153 | + | if err := os.Symlink("pipe", filepath.Join(ws, "pipelink")); err != nil { | |||
| 154 | + | t.Fatal(err) | |||
| 155 | + | } | |||
| 156 | + | only, err := hashFilesIn(t.Context(), ws, "/h", []string{"a.txt"}, false) | |||
| 157 | + | if err != nil || only == "" { | |||
| 158 | + | t.Fatalf("%q %v", only, err) | |||
| 159 | + | } | |||
| 160 | + | for _, tc := range []struct { | |||
| 161 | + | patterns []string | |||
| 162 | + | follow bool | |||
| 163 | + | }{ | |||
| 164 | + | {[]string{"**"}, false}, {[]string{"**"}, true}, {[]string{"pipe"}, false}, {[]string{"pipe"}, true}, | |||
| 165 | + | {[]string{"pipelink"}, false}, {[]string{"pipelink"}, true}, | |||
| 166 | + | } { | |||
| 167 | + | done := make(chan struct{}) | |||
| 168 | + | var got string | |||
| 169 | + | var err error | |||
| 170 | + | go func() { | |||
| 171 | + | defer close(done) | |||
| 172 | + | got, err = hashFilesIn(t.Context(), ws, "/h", tc.patterns, tc.follow) | |||
| 173 | + | }() | |||
| 174 | + | select { | |||
| 175 | + | case <-done: | |||
| 176 | + | case <-time.After(5 * time.Second): | |||
| 177 | + | t.Fatalf("%q follow=%v: hangs on a FIFO", tc.patterns, tc.follow) | |||
| 178 | + | } | |||
| 179 | + | if err != nil { | |||
| 180 | + | t.Errorf("%q follow=%v: %v", tc.patterns, tc.follow, err) | |||
| 181 | + | } | |||
| 182 | + | if tc.patterns[0] == "**" && got != only { | |||
| 183 | + | t.Errorf("%q follow=%v: %q, want a.txt alone %q", tc.patterns, tc.follow, got, only) | |||
| 184 | + | } | |||
| 185 | + | if tc.patterns[0] != "**" && got != "" { | |||
| 186 | + | t.Errorf("%q follow=%v: %q, want no file", tc.patterns, tc.follow, got) | |||
| 187 | + | } | |||
| 188 | + | } | |||
| 189 | + | } | |||
| 190 | + | ||||
| 142 | type officialResult struct { | 191 | type officialResult struct { | ||
| 143 | sum string | 192 | sum string | ||
| 144 | failed bool | 193 | failed bool | ||