os: `Root.MkdirAll` can return "file exists" when called concurrently on the same path

[database64128]

Go version

go version go1.25.0 linux/amd64

Output of go env in your module/workspace:

AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v3'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/home/database64128/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/database64128/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2391249113=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/home/database64128/repos/modpack-dl-go/go.mod'
GOMODCACHE='/home/database64128/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/database64128/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='on'
GOTELEMETRYDIR='/home/database64128/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/usr/lib/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.25.0'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

I have a file downloading program that fetches a file list and spins up 32 worker goroutines to download all listed files. Before downloading, it first tries to open or create the file at the destination to see if it can be skipped:

// createFile creates the file at the given path.
// The parent directory will be created if it doesn't exist.
// It returns the opened created file or an error.
func createFile(path string) (*os.File, error) {
	f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0644)
	if err != nil {
		if err = os.MkdirAll(filepath.Dir(path), 0755); err != nil {
			return nil, err
		}
		return os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0644)
	}
	return f, nil
}

After I upgraded to Go 1.25 and converted it to use os.Root (#67002):

// createFile creates the file at the given path.
// The parent directory will be created if it doesn't exist.
// It returns the opened created file or an error.
func createFile(root *os.Root, path string) (*os.File, error) {
	f, err := root.OpenFile(path, os.O_RDWR|os.O_CREATE, 0644)
	if err != nil {
		if err = root.MkdirAll(filepath.Dir(path), 0755); err != nil {
			return nil, err
		}
		return root.OpenFile(path, os.O_RDWR|os.O_CREATE, 0644)
	}
	return f, nil
}

What did you see happen?

createFile now has a chance of returning an error file exists. This is likely caused by:

go/src/os/root_openat.go

Lines 126 to 136 in ffc85ee

	fd, err := rootOpenDir(parent, name)
	switch err.(type) {
	case nil, errSymlink:
	return fd, err
	}
	if try > 0 || !IsNotExist(err) {
	return 0, &PathError{Op: "openat", Err: err}
	}
	if err := mkdirat(parent, name, perm); err != nil {
	return 0, &PathError{Op: "mkdirat", Err: err}
	}

The error path at L135 needs to double-check whether the directory exists, like os.MkdirAll does:

go/src/os/path.go

Lines 54 to 65 in ffc85ee

	// Parent now exists; invoke Mkdir and use its result.
	err = Mkdir(path, perm)
	if err != nil {
	// Handle arguments like "foo/." by
	// double-checking that directory doesn't exist.
	dir, err1 := Lstat(path)
	if err1 == nil && dir.IsDir() {
	return nil
	}
	return err
	}
	return nil

Otherwise, if the directory was created between rootOpenDir and mkdirat by another goroutine, os.Root.MkdirAll would fail.

I can send a CL to fix this if you are OK with this approach. @neild

Update: Opened CL 698215.

What did you expect to see?

os.Root.MkdirAll does not return any error.

[gabyhelp]

Related Issues

• os: document that OpenFile Create do not create file if directory do not exist #69836 (closed)
• os: MkdirTemp rare error message has wrong path #75012 (closed)
• os: `Root.OpenRoot` sets incorrect name, losing prefix of original root #73868
• os: Root.Mkdir creates directories with zero permissions on OpenBSD #73559 (closed)
• os: MkdirAll exits without creating all folders #20289 (closed)
• os: Race condition in (* os.File).readdir() #71496 (closed)
• MkdirAll() returns unhelpful error message if the parent directory is / #1637 (closed)

Related Code Changes

• os: handle trailing spaces case on MkdirAll
• os: don't check for IsExist in MkdirAll example
• os: make MkdirAll support volume names

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[gopherbot]

Change https://go.dev/cl/698215 mentions this issue: os: fix Root.MkdirAll to handle race of directory creation

[database64128]

@gopherbot please open backport issues. The fix is just an additional error check. The workaround involves calling os.Root.MkdirAll in a loop, which doesn't look very safe:

for {
	if err := root.MkdirAll(path, 0755); err != nil {
		if errors.Is(err, os.ErrExist) {
			continue
		}
		return nil, err
	}
	break
}

You can't put a limit on the number of retries, unless you know in advance how many path components are there.

[gopherbot]

Backport issue(s) opened: #75115 (for 1.24), #75116 (for 1.25).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

[cagedmantis]

cc @neild @rsc @ianlancetaylor @bradfitz @griesemer