Skip to content
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: return value of Fd can be closed unexpectedly by finalizer #9046

Closed
gopherbot opened this issue Nov 2, 2014 · 6 comments
Closed

os: return value of Fd can be closed unexpectedly by finalizer #9046

gopherbot opened this issue Nov 2, 2014 · 6 comments
Milestone

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 2, 2014

by 2852914@wessie.info:

What does 'go version' print?

go version devel +53ebc70d4e9f Sat Nov 01 08:28:09 2014 -0700 linux/amd64
&
go version go1.3.3 linux/amd64

What steps reproduce the problem?

http://play.golang.org/p/gzyJ0vJPEK

What happened?

bad file descriptor after a few loops (i <= 500)

What should have happened instead?

loop indefinitely, the reproduction has no success case and only exits on the bad file
descriptor.

Please provide any additional information below.

It seems tty isn't live after the log.Fatal line, and err isn't live on the log.Fatal
line

output of `go build -gcflags "-live -l" dom.go`:

tip:
./dom.go:24: live at call to convI2E: autotmp_0007 autotmp_0000 tty
./dom.go:24: live at call to writebarrieriface: autotmp_0007 autotmp_0000 tty
./dom.go:24: live at call to Fatal: autotmp_0000 tty
./dom.go:38: live at call to convT2E: autotmp_0003 autotmp_0007
./dom.go:38: live at call to writebarrieriface: autotmp_0003 autotmp_0007
./dom.go:38: live at call to convT2E: autotmp_0003 autotmp_0007
./dom.go:38: live at call to writebarrieriface: autotmp_0003 autotmp_0007
./dom.go:38: live at call to Printf: autotmp_0003

1.3.3:
./dom.go:24: live at call to convI2E: autotmp_0006 autotmp_0000 tty
./dom.go:24: live at call to Fatal: autotmp_0000 tty
./dom.go:38: live at call to Printf: autotmp_0002
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 2, 2014

Comment 1:

Labels changed: added repo-main, release-go1.4.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 2, 2014

Comment 2:

I agree that the problem is that tty is not live.  The *os.File to which it points is
eventually collected, and the finalizer closes the file descriptor.  There is nothing
keeping tty live, so it is hard to blame the compiler.  This would be more obvious if
the function were
func ttyFd() uintptr {
    tty, _ := os.Open("/dev/tty")
    return tty.Fd()
}
I think our choices are 1) document that you must be careful to keep the *os.File live
if you call the Fd method; 2) have the Fd method clear the finalizer, and document that
the caller of Fd is responsible for closing the descriptor, somehow--actually this would
probably require adding a new function to close a descriptor.

Labels changed: added release-go1.4maybe, removed release-go1.4.

@davecheney
Copy link
Contributor

@davecheney davecheney commented Nov 2, 2014

Comment 3:

This bug is going on my list of why os.File shouldn't have a finaliser.
I also like the idea that calling .Fd() disables the finaliser.
@rsc
Copy link
Contributor

@rsc rsc commented Nov 4, 2014

Comment 4:

Easy fix: keep tty live, such as by calling tty.Close when you are done with the fd.
If you really want just an fd, use syscall.Open.

Status changed to WorkingAsIntended.

@gopherbot
Copy link
Author

@gopherbot gopherbot commented Nov 4, 2014

Comment 5:

CL https://golang.org/cl/162680043 mentions this issue.
@rsc
Copy link
Contributor

@rsc rsc commented Nov 6, 2014

Comment 6:

This issue was closed by revision 1cdd9b4.

Status changed to Fixed.

@gopherbot gopherbot added fixed labels Nov 6, 2014
@rsc rsc added this to the Go1.4 milestone Apr 14, 2015
@rsc rsc removed the release-go1.4maybe label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 20, 2018
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.