The docs of File.Readdir, File.ReadDir, and File.Readdirnames make no such guarantee of that. Any particular reasons why we should make it concurrent-safe? Calling ReadDir on the same *os.File simultaneously doesn't seem like a typical scenario.

The docs don't mention concurrency safety, but (and someone who was around when the os package was created can confirm or deny this) I suspect that's because the assumption was that *os.File methods are obviously supposed to be safe to call from multiple goroutines. An *os.File, per its documentation, represents an open file descriptor, and Unix FD operations are always concurrency-safe.

If ReadDir, or any other method, is not supposed to be safe to call concurrently, the docs definitely should mention that.

(Of course, methods which modify file state are inherently unsafe--Read and Write modify the current file position, for example. But I'd expect ReadAt to be safe.)

File.ReadDir can't be concurrent safe when doing partial reads, i.e. calling File.ReadDir(n) on a directory with more than n entries. It uses an internal state to keep track of the current entry position so the next File.ReadDir(n) call returns the following n entries.

On the other hand, we could special the n <= 0 case, which is documented to return all the remaining entries, to make it concurrent-safe, as it doesn't need to keep track of read file entries between calls.

In general, unless otherwise documented, standalone functions should be designed to be concurrency-safe, but methods of a type should not be assumed to be concurrency safe. So os.Open must be safe, but Read or ReadDir need not be. I don't think there's a bug or even a doc issue here.

We do go to some trouble to ensure that concurrent calls to ReadAt and WriteAt will work. It is of course impossible to support concurrent calls to Read and Write. So I think the only question is whether we want Readdir(-1) to be concurrency-safe. I don't have a strong opinion myself.

Concurrent calls to Write in append mode might work? I don't recall what, if any, guarantees the OS offers there.

I think Readdir(-1) should either be concurrency-safe or explicitly documented as unsafe. I lean towards safe, but not strongly.

Should there be a version of File.ReadDir that returns an iterator? #61897 states that os.ReadDir won't get an iterator variant because it sorts its result, but File.ReadDir returns unsorted results.

I was coming here to file a similar issue, but found this first. My real-world use case is concurrent calls to File.Readdirnames and File.Close. There's a goroutine polling for updates to a directory and the graceful shutdown code calls Close on the file from a different goroutine. I expected the same semantics as a concurrent Unix FD read and close.

[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x4e18ea]

goroutine 5044070 [running]:
os.(*File).readdir(0xc00039a0e0, 0xffffffffffffffff?, 0x0)
        /usr/local/go/src/os/dir_unix.go:81 +0x24a
os.(*File).Readdirnames(0x8a68ed?, 0xbdd000?)
        /usr/local/go/src/os/dir.go:70 +0x1a
<my code ommitted>

[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x4e18ea]

For the record: assuming you're using go1.22.0, this crash indicates a race on the File.dirInfo.buf buffer.

There are races on File.dirinfo and dirInfo.buf.

https://github.com/golang/go/blob/go1.22.1/src/os/file_unix.go#L310-L313

func (file *file) close() error {
	// ...
	if file.dirinfo != nil {
		file.dirinfo.close()
		file.dirinfo = nil
	}
	// ...
}

https://github.com/golang/go/blob/go1.22.1/src/os/dir_unix.go#L37-L42

func (d *dirInfo) close() {
	if d.buf != nil {
		dirBufPool.Put(d.buf)
		d.buf = nil
	}
}

https://github.com/golang/go/blob/go1.22.1/src/os/dir_unix.go#L46-L50

func (f *File) readdir(n int, mode readdirMode) (names []string, dirents []DirEntry, infos []FileInfo, err error) {
	if f.dirinfo == nil {
		f.dirinfo = new(dirInfo)
		f.dirinfo.buf = dirBufPool.Get().(*[]byte)
	}
	d := f.dirinfo
	// ... many uses of d.buf ...
}

Leaning towards os.(*File).ReadDir's behaviour being correctly not specified:

By default, programmers should expect that a type is safe for use only by a single goroutine at a time.

Cf. https://go.dev/doc/comment#type [2024-04-04]

By default, programmers can assume that a top-level function is safe to call from multiple goroutines; this fact need not be stated explicitly.

On the other hand, as noted in the previous section, using an instance of a type in any way, including calling a method, is typically assumed to be restricted to a single goroutine at a time.

Cf. https://go.dev/doc/comment#func [2024-04-04]

It might not hurt to make an exception to the rule here and redundantly document the lack of concurrency safety, because many readers may wrongly assume that Read, Write, ReadDir, and other methods named for UNIX system calls are nothing more than wrappers around those system calls.

@neild

(Of course, methods which modify file state are inherently unsafe--Read and Write modify the current file position, for example. But I'd expect ReadAt to be safe.)

Concurrency is built into io.ReaderAt's contract, not a property of some external Unix-y interpretation of how a file ought to behave:

Clients of ReadAt can execute parallel ReadAt calls on the same input source.

Cf. https://pkg.go.dev/io@go1.22.2#ReaderAt [2024-04-04]

@jfrech

Concurrency is built into io.ReaderAt's contract, not a property of some external Unix-y interpretation of how a file ought to behave:

The documentation for the os.File struct is "File represents an open file descriptor." I think that carries a bit of Unix-y interpretations with it.

Change https://go.dev/cl/578322 mentions this issue: os: document concurrency unsafety of File, Readdir et al

Note: the subject of the fix as merged was "os: make File.Readdir et al concurrency-safe".