-
Notifications
You must be signed in to change notification settings - Fork 17.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
os: document file descriptor ownership semantics for os.NewFile() #43863
Labels
Documentation
Issues describing a change to documentation.
FrozenDueToAge
NeedsFix
The path to resolution is known, but the work has not been done.
Milestone
Comments
vikmik
changed the title
os: document os.File ownership semantics for os.NewFile()
os: document file descriptor ownership semantics for os.NewFile()
Jan 23, 2021
vikmik
added a commit
to vikmik/go
that referenced
this issue
Jan 23, 2021
NewFile requires the file descriptor to be either closed through the returned File instance, or to stay valid at least until the finalizer runs during garbage collection. These requirements are easily violated when file descriptors are closed via unix.Close, or when the File returned by NewFile is garbage collected while the file descriptor is still valid and in use. This commit adds further documentation for NewFile, making it explicit that the file descriptor must only be closed via the returned File. Fixes golang#43863
Not sure if documentation PRs are looked at without discussion here - sorry if this isn't the case. I posted #43867 / https://golang.org/cl/286032 |
Change https://golang.org/cl/286032 mentions this issue: |
vikmik
added a commit
to vikmik/go
that referenced
this issue
Jan 24, 2021
NewFile requires the file descriptor to be either closed through the returned File instance, or to stay valid at least until the finalizer runs during garbage collection. These requirements are easily violated when file descriptors are closed via unix.Close, or when the File returned by NewFile is garbage collected while the file descriptor is still valid and in use. This commit adds further documentation for NewFile, making it explicit that the file descriptor must only be closed via the returned File. Fixes golang#43863
vikmik
added a commit
to vikmik/go
that referenced
this issue
Jan 24, 2021
NewFile requires the file descriptor to be either closed through the returned File instance, or to stay valid at least until the finalizer runs during garbage collection. These requirements are easily violated when file descriptors are closed via unix.Close, or when the *File returned by NewFile is garbage collected while the underlying file descriptor is still in use. This commit adds further documentation for NewFile and Fd, making it explicit that using naked file descriptors is subject to constraints due to garbage collection of File objects. Fixes golang#43863
toothrot
added
the
NeedsFix
The path to resolution is known, but the work has not been done.
label
Jan 25, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
Documentation
Issues describing a change to documentation.
FrozenDueToAge
NeedsFix
The path to resolution is known, but the work has not been done.
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
I am not providing a repro case here, as this is dependent on GC pressure & is intended behavior
What I did: I used
os.NewFile
with a file descriptor owned by a different piece of code. To illustrate:This seems to be intended behavior because of: https://github.com/golang/go/blob/master/src/os/file_unix.go#L178
The GC behavior is documented on
os.File.Fd()
: https://github.com/golang/go/blob/master/src/os/file_unix.go#L65What did you expect to see?
I expected to see a comment on
os.NewFile
warning me that the naked file descriptor can only be used while the returnedos.File
isn't garbage-collectedWhat did you see instead?
Documentation isn't explicit (unless I was looking at the wrong place?),
os.NewFIle
doesn't mention garbage collection at all.This triggered a difficult-to-reproduce/diagnose bug in my service - I had to go and read the Golang source code to find the root cause.
I'll be happy to open a PR to fix the documentation if this issue seems legit.
Thanks!
The text was updated successfully, but these errors were encountered: