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

proposal: os: support disposing of File without closing its fd #24215

Closed
lucab opened this issue Mar 2, 2018 · 8 comments
Closed

proposal: os: support disposing of File without closing its fd #24215

lucab opened this issue Mar 2, 2018 · 8 comments

Comments

@lucab
Copy link
Contributor

lucab commented Mar 2, 2018

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

Go 1.10

Summary

os.File can wrap existing file-descriptors (fd), via NewFile(), in a file object. As with all File objects, this automatically closes the fd at GC time (i.e. via a private finalizer). This works well when taking ownership of an existing fd, but makes temporary borrowing of an fd impossible/racey: any further Newfile usage may or may not work, depending on relative timing with the previous finalizer.

This is a proposal for an explicit method to signal the end of fd borrowing, without closing it on GC. This is to model the non-ownership case.

Usecase

As a practical use-case, let's pick a Go server which is socket activated by systemd.
In this case, the fd is passed (and indexed) by systemd to the child Go server, and any File object can temporarily borrow it via NewFile wrapping:

fp := os.NewFile(uintptr(fd), "socket")
listener, _ := net.FileListener(fp)
...
listener.Close()

At the end of this snippet, users may want re-use the external fd but doing so is unsafe in a subtle way: as soon as the File object for fp is not referenced anymore, GC can decide to run the internal finalizer at any point, and then fd will suddenly be closed.

Note: as per public API doc, fp and listener lifespans are not strictly coupled.

Proposal

To avoid racing with the finalizer, introduce a method to signal the end of a fd-borrowing span.
Name and signature could be as follow:

// Relinquish returns the underlying file-descriptor and
// invalidates this `*File` instance.
//
// Once this method returns without errors, the `fd` is
// left intact (i.e. not closed) and can be further re-used.
// Instead, further operations on the `*File` receiver are
// not possible.
// This is mainly used to temporarily borrow an existing `fd`
// via `NewFile`, then eventually relinquishing it without
// closing.
func (f *File) Relinquish() (uintptr, error) { ... }

Semantics would be as follow:

  • explicit error is returned for invalid cases (e.g. a nil File.file)
  • the original blocking/non-blocking flag is reset to the original mode on the fd before returning (i.e. set to f.nonblock)
  • the internal File.file finalizer is reset to nil (i.e. no closing on GC)
  • internal File.file is reset to nil (i.e. further operations on File are invalid)

/cc @squeed

@ianlancetaylor ianlancetaylor changed the title os: support disposing of File without closing its fd proposal: os: support disposing of File without closing its fd Mar 2, 2018
@gopherbot gopherbot added this to the Proposal milestone Mar 2, 2018
@lucab
Copy link
Contributor Author

lucab commented Mar 2, 2018

Barring any negative feedback on this proposal in the upcoming weeks, I plan to tackle this further with an implementation+tests (to scratch my own itch).

@artyom
Copy link
Member

artyom commented Mar 2, 2018

@lucab have you tried runtime.KeepAlive for this use case? Yours seems quite similar to the documented example.

@lucab
Copy link
Contributor Author

lucab commented Mar 2, 2018

@artyom yes, I'm aware of Keepalive but no this isn't the same usecase: in the snippet above, fp can/should go out of scope early (i.e. being local to a function scope) and separately from the listener (see note).
Library code won't have any need to expose fp nor to do bookeeping on its lifetime, only the file-descriptor has to be preserved.

@crvv
Copy link
Contributor

crvv commented Mar 4, 2018

I think you can use a duplicated fd in os.NewFile().
Like https://golang.org/pkg/net/#TCPConn.File

@lucab
Copy link
Contributor Author

lucab commented Mar 4, 2018

@crvv thanks for your feedback! Yes, dup-ing the fd was also considered before proposing this, but it is merely a resource-heavy workaround for the borrowing case.
In the above snippet and example, it would introduce a 2x fd usage upfront for sockets passed by systemd (and fd numbres are limited resources) and it will duplicate functionality already taken care of by FileListener/TCPConn.File/etc.

@rsc
Copy link
Contributor

rsc commented Mar 5, 2018

This seems very very niche. Why not just assign the file to a map of things you don't want to close? It's hard to believe this comes up often enough to justify the addition of new methods in the library itself.

FWIW I've used systemd sockets too and the programs seem to work fine just getting them once at the start of the program and holding onto them, instead of picking up and dropping the fd repeatedly.

@lucab
Copy link
Contributor Author

lucab commented Mar 12, 2018

@rsc I understand that my example may look very niche, and that with custom applications it is easy to workaround by performing additional manual book-keeping (as all workarounds above show).
However, the core of my usecase goes a bit further than that:

  • as a go-systemd maintainer, I can't do the book-keeping for my consumers. We recently realized this subtle race exists in the example we have. As it is transparent to consumers, I would ideally like to have this fixed internally. Returning duplicate sets of (e.g.) Files+Listeners to users seems un-ergonomic and confusing.
  • as a stdlib consumer, I am missing the correct way of borrowing a file-descriptor. The path from a fd to a (e.g.) Listener forcefully goes through a temporary File object, all of which is stdlib API. I am not sure this can cleanly fixed outside of stdlib without internals-duplication and type aliasing (if even).
  • as a system developer, passing fd around is quite common when doing IPC in UNIX world and this race is quite subtle. I can't really say how many other people may need this (or how many realized their code can be racy), but having a clearer story around borrowing vs owning a fd would be IMHO generally helpful
  • as a Go developer, I'd like to have the same expressiveness I have in other system-languages. For example, D has a dedicated constructor for the borrowing case, while Rust has a dedicated method to consume the wrapping object without closing the underlying fd.

Of course I understand stdlib design and maintenance is a nuanced topic, so I'm just trying to clarify what may look like an odd proposal.

@ianlancetaylor
Copy link
Member

This still seems too niche for the Go standard library. Go does provide the golang.org/x/sys/unix package if you want to do all your file descriptor management yourself.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants