Don't use syscall.Close() on inherited fd #29
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Inherited file descriptor is encapsulated into a `File` using `os.NewFile()`. There is no `dup()` done by Go in this case and a finalizer will close the `File` (and the file descriptor if the file wasn't already closed) when it becomes out of reach. Using `syscall.Close()` will close the file descriptor but not the `File`. At some point, the finalizer will kick in, notice the file hasn't been closed and close again the file descriptor. If the file descriptor was reassigned to something else, we close an unrelated file descriptor. Tested with Go 1.7. Source code shows there is no `dup()` done when using `NewFile()`: - Go 1.7: https://github.com/golang/go/blob/release-branch.go1.7/src/os/file_unix.go#L50 - Go 1.6: https://github.com/golang/go/blob/release-branch.go1.6/src/os/file_unix.go#L50 - Go 1.5: https://github.com/golang/go/blob/release-branch.go1.5/src/os/file_unix.go#L48 - Go 1.4: https://github.com/golang/go/blob/release-branch.go1.4/src/os/file_unix.go#L40
|
In fact, this is a duplicate of #28... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Inherited file descriptor is encapsulated into a
Fileusingos.NewFile(). There is nodup()done by Go in this case and a finalizer will close theFile(and the file descriptor if the file wasn't already closed) when it becomes out of reach. Usingsyscall.Close()will close the file descriptor but not theFile. At some point, the finalizer will kick in, notice the file hasn't been closed and close again the file descriptor. If the file descriptor was reassigned to something else, we close an unrelated file descriptor.Tested with Go 1.7. Source code shows there is no
dup()done when usingNewFile():