Skip to content

Commit e9d5f81

Browse files
committed
Fix macOS tmpdir cleanup race caused by system daemons creating ~/Library
On macOS, system daemons (cfprefsd, lsd) automatically create $HOME/Library/ for any process whose HOME they observe. Since the build env sets HOME=tmpDir, these daemons can recreate Library/ inside a target's temporary directory during or just after cleanup, causing os.RemoveAll to fail with ENOTEMPTY. Handle this with a platform-specific cleanTmpDir that detects the Library/ directory on ENOTEMPTY and retries removal with bounded backoff. Non-Darwin platforms get a zero-overhead passthrough.
1 parent 71ec53b commit e9d5f81

3 files changed

Lines changed: 103 additions & 2 deletions

File tree

src/build/build_step.go

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -407,8 +407,10 @@ func buildTarget(state *core.BuildState, target *core.BuildTarget, runRemotely b
407407
}
408408
// Clean up the temporary directory once it's done.
409409
if state.CleanWorkdirs {
410-
if err := fs.RemoveAll(target.TmpDir()); err != nil {
411-
log.Warning("Failed to remove temporary directory for %s: %s", target.Label, err)
410+
tmpDir := target.TmpDir()
411+
if err := cleanTmpDir(tmpDir); err != nil {
412+
log.Warning("Failed to remove temporary directory for %q: %v", target.Label, err)
413+
logStaleDirectoryContents(tmpDir)
412414
}
413415
}
414416
if outputsChanged {
@@ -1231,3 +1233,30 @@ func build(state *core.BuildState, target *core.BuildTarget, inputHash []byte) (
12311233
}
12321234
return nil, fmt.Errorf("Persistent workers are no longer supported, found worker command: %s", workerCmd)
12331235
}
1236+
1237+
// logStaleDirectoryContents recursively logs the remaining contents of a
1238+
// directory that os.RemoveAll failed to remove. This helps diagnose what is
1239+
// still holding files open or creating files during cleanup.
1240+
func logStaleDirectoryContents(dir string) {
1241+
err := filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error {
1242+
if err != nil {
1243+
log.Debug(" stale (walk error): %s: %v", path, err)
1244+
return nil
1245+
}
1246+
if path == dir {
1247+
return nil
1248+
}
1249+
rel, _ := filepath.Rel(dir, path)
1250+
info, ierr := d.Info()
1251+
if ierr != nil {
1252+
log.Debug(" stale: %s (stat failed: %v)", rel, ierr)
1253+
return nil
1254+
}
1255+
log.Debug(" stale: %s (type=%s, size=%d, mtime=%s)",
1256+
rel, d.Type(), info.Size(), info.ModTime().Format("15:04:05.000"))
1257+
return nil
1258+
})
1259+
if err != nil {
1260+
log.Debug(" could not walk remaining contents of %s: %v", dir, err)
1261+
}
1262+
}

src/build/clean_tmpdir_darwin.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package build
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"os"
7+
"path/filepath"
8+
"syscall"
9+
"time"
10+
11+
"github.com/thought-machine/please/src/fs"
12+
)
13+
14+
// cleanTmpDir removes a target's temporary build directory.
15+
//
16+
// On macOS, the operating system automatically creates a ~/Library directory
17+
// structure for every HOME directory it observes in use by a process. This is
18+
// documented in Apple's File System Programming Guide:
19+
//
20+
// https://developer.apple.com/library/archive/documentation/FileManagement/Conceptual/FileSystemProgrammingGuide/MacOSXDirectories/MacOSXDirectories.html
21+
//
22+
// Several per-user system daemons (cfprefsd, lsd, etc.) monitor process HOME
23+
// values and lazily create $HOME/Library/ and its subdirectories (Preferences,
24+
// Caches, etc.) for any new HOME path they observe. This creation is
25+
// asynchronous and can occur after the process that triggered it has already
26+
// exited.
27+
//
28+
// Since the build environment sets HOME=tmpDir (to isolate each target's build
29+
// from the user's home directory), these daemons may create a Library/
30+
// directory inside the target's tmpDir during or shortly after the build
31+
// completes. When os.RemoveAll deletes the tmpDir contents and then attempts
32+
// to remove the now-empty directory, a daemon may have re-created Library/ in
33+
// the interim, causing the removal to fail with ENOTEMPTY.
34+
//
35+
// We handle this by detecting the Library/ directory after an ENOTEMPTY
36+
// failure, removing it, and retrying with bounded backoff to allow the daemons
37+
// to settle.
38+
func cleanTmpDir(tmpDir string) error {
39+
err := fs.RemoveAll(tmpDir)
40+
if err == nil || !errors.Is(err, syscall.ENOTEMPTY) {
41+
return err
42+
}
43+
44+
libDir := filepath.Join(tmpDir, "Library")
45+
const maxAttempts = 3
46+
for attempt := range maxAttempts {
47+
if attempt > 0 {
48+
time.Sleep(time.Second)
49+
}
50+
if info, serr := os.Stat(libDir); serr != nil || !info.IsDir() {
51+
// Library/ is not the problem; return the original error.
52+
return err
53+
}
54+
os.RemoveAll(libDir)
55+
if rerr := os.Remove(tmpDir); rerr == nil {
56+
return nil
57+
} else if !errors.Is(rerr, syscall.ENOTEMPTY) {
58+
return rerr
59+
}
60+
}
61+
return fmt.Errorf("failed to remove %s after %d attempts to clean macOS Library dir: %w", tmpDir, maxAttempts, err)
62+
}

src/build/clean_tmpdir_default.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
//go:build !darwin
2+
3+
package build
4+
5+
import "github.com/thought-machine/please/src/fs"
6+
7+
// cleanTmpDir removes a target's temporary build directory.
8+
func cleanTmpDir(tmpDir string) error {
9+
return fs.RemoveAll(tmpDir)
10+
}

0 commit comments

Comments
 (0)