search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.08k stars 17.68k forks source link

os: document concurrency properties for File methods #56043

Open bcmills opened 2 years ago

bcmills commented 2 years ago

What version of Go are you using (go version)?

https://pkg.go.dev/os@go1.19.1

Does this issue reproduce with the latest release?

Yes.

What did you do?

For #50436, I'm attempting to unblock reads on a *File returned by os.Pipe while it is being read concurrently by a user-controlled goroutine. The user-controlled goroutine may legitimately call the Close method, and may expect to be able to access other methods (such as SetDeadline) via type-assertion.

What did you expect to see?

Given #6270, #7970, #9307, #17647 and https://go.dev/cl/65490, I expected the documentation for `*os.File to describe which methods are safe to invoke concurrently and under what conditions.

What did you see instead?

The only mention of concurrency I could find in https://pkg.go.dev/os@go1.19.1 says this:

Note: The maximum number of concurrent operations on a File may be limited by the OS or the system. The number should be high, but exceeding it may degrade performance or cause other issues.

That seems to imply that at least some of the File methods may be invoked concurrently, but isn't explicit about which ones.

I see unsynchronized writes in the Close implementation on both unix and plan9 (but maybe not windows?); it's not obvious to me which other methods are or aren't safe.

(CC @rsc @ianlancetaylor @robpike from previous *File race conditions.)

robpike commented 2 years ago

Unless we're willing to put locks everywhere, I'm not sure what we can say beyond saying to read the documents for the operating system.

gopherbot commented 2 years ago

Change https://go.dev/cl/438347 mentions this issue: os: add test that concurrent calls to Close on a pipe are OK

ianlancetaylor commented 2 years ago

As it happens, we actually already have all the required locks for regular files or pipes, on all systems other than Plan 9. I've already sent CL 438347 to fix Plan 9.

We have the locks because we need them anyhow for the runtime poller.

The current exception is for directories. For directories os files have a dirInfo field. There are no locks around access to that field.

gopherbot commented 2 years ago

Change https://go.dev/cl/439195 mentions this issue: os/exec: remove protection against a duplicate Close on StdinPipe

gopherbot commented 2 years ago

Change https://go.dev/cl/439595 mentions this issue: os: ensure that concurrent I/O and Close on a pipe are OK