os: add File.SyscallConn method to permit frobbing file descriptor #24331

npat-efault · 2018-03-10T02:52:28Z

Since go 1.9, os.Files are backed by a mechanism similar to netpoll, whenever possible. This means that you can set read and write deadlines on them, and that you can safely Close() a file from another goroutine while Read() and / or Write() calls are active.

For working with device nodes (/dev/XXX files, like serial ports /dev/ttySx, but not only) this is very useful.

Sadly it turns out one cannot use this new feature, for a trivial and equally frustrating reason:

Occasionally one needs to access the underlying file-descriptor of a device in order to do a trivial ioctl() or something similar. But calling File.Fd(), sets the underlying filedes to blocking mode and disables (forever) all the cool poller support (deadlines, etc).

One cannot turn the filedes to nonblocking mode himself, because he would also need to set internal File / pfd attributes which he has no access to.

Alternatively, one cannot start with a filedes (i.e. use syscall.Open) and convert it to a file, using NewFile() since Files created with NewFile are not considered pollable.

So please: Provide a way to get a File's underlying filedes without setting it to non-blocking mode, and (most importantly) without loosing all the poller goodies. A new method like File.RawFd() would be perfectly adequate and trivial to implement.

I believe my case (accessing pollable device nodes) is exactly one of those for which the poller support was added to File's. So I should be able to use it.

The text was updated successfully, but these errors were encountered:

npat-efault · 2018-03-10T03:11:06Z

An alternative solution would be for NewFile() to test if the given fd is in non-blocking mode, in which case it would convert it to a pollable file. Otherwise, it would convert it to a non-pollable file, as it, unconditionally, does today. Actually, doing both would be nice...

npat-efault · 2018-03-10T03:21:28Z

If you want, I could make the changes to os.NewFile() and / or add the File.RawFd() method myself and submit a patch...

odeke-em · 2018-03-11T01:22:10Z

@npat-efault thank you for filing and for articulating your motivations for this issue.

You might be interested in this issue #22939 that was accepted for Go1.11 in which the proposal was to add an API to construct an *os.File with a non-blocking file descriptor. Perhaps that issue might solve your problem without having to add another method, plus it is just waiting on implementation so perhaps you might be interested?

In regards to #24331 (comment)

An alternative solution would be for NewFile() to test if the given fd is in non-blocking mode, in which case it would convert it to a pollable file. Otherwise, it would convert it to a non-pollable file, as it, unconditionally, does today. Actually, doing both would be nice...

@ianlancetaylor had the exact same thoughts in #22939 (comment)

Perhaps if os.NewFile sees that the descriptor is already in non-blocking mode, it should try adding it to the poller.

And now to the question for clarity, your issue says
os: File.Fd() sets underlying filedes to non-blocking.
However the context of it is that we are setting filedes to blocking mode by invoking *File.Fd().

Did you mean to say instead either of these options below?

os: File.Fd() set underlying filedes to blocking mode
proposal: os: provide a method on *File to access filedes without setting it to blocking mode
proposal: os: add a *File.RawFd method

as · 2018-03-11T01:45:41Z

What about the users that rely on the current behavior of file.Fd()?

npat-efault · 2018-03-11T07:44:14Z

Sorry, this was a typo. I fixed the title.

My proposal is:

Leave File.Fd() as is, since users may depend on it

and one or, better, both of:

Change NewFile() to convert an already-nonblocking filedes to a pollable file
Add a File.RawFd() method that returns the underlying filedes without any conversion from / to blocking / non-blocking

Technically 1 above could be said that it breaks backwards compatibility (in some sense), but if we agree that this break is not one we care about (the original behavior was not useful anyway), and if we agree that this is the way to go, I could post a (or two for 1 and 2) CLs in a couple of days or sooner.

If we don't want to break backward compatibility in no way whatsoever by changing the behavior of NewFile(), then we could define a NewFile1() with the new behavior (yes, it's not the prettiest thing, but if you are so strongly set about backwards compatibility, you have to abide a bit of ugliness).

gopherbot · 2018-03-11T18:33:03Z

Change https://golang.org/cl/100077 mentions this issue: os: NewFile called with nonblock fd, tries to return pollable file

npat-efault · 2018-03-11T18:46:41Z

Submitted CL https://golang.org/cl/100077 for your consideration...

andybons · 2018-03-11T19:10:08Z

/cc @ianlancetaylor for a decision

rsc · 2018-03-26T20:35:51Z

I thought we'd done (1) ("Change NewFile() to convert an already-nonblocking filedes to a pollable file") already. /cc @ianlancetaylor

rsc · 2018-03-26T20:37:36Z

Re (2) I think adding SyscallConn is probably the next step instead of RawFd, but we don't need that yet.

npat-efault · 2018-04-03T12:24:06Z

@rsc

I agree... though a bit cumbersome to use, SyscallCon() is the "right way" to provide access to the underlying fd for misc operations (ioctls and stuff). This is how it's done for network connections, and there is no reason for it to be different for other "connection-like" file-descriptors. Furthermore it provides synchronization against concurrent Close() calls (which is something the user should provide himself in the RawFd() case).

I could start looking into the implementation of this, if there are no other takers...

If NewFile is called with a file descriptor that is already set to non-blocking mode, it tries to return a pollable file (one for which SetDeadline methods work) by adding the filedes to the poll/netpoll mechanism. If called with a filedes in blocking mode, it returns a non-pollable file, as it always did. Fixes #22939 Updates #24331 Change-Id: Id54c8be1b83e6d35e14e76d7df0e57a9fd64e176 Reviewed-on: https://go-review.googlesource.com/100077 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>

zx2c4 · 2018-04-19T06:12:04Z

I'm currently having to use gross syscall.Select signaling to work around concurrent .Read() and .Close() when dealing with a /dev/net/tun that I call .Fd() on. With #22939 taken care of, having .RawFd() is the remaining issue before I can remove the syscall.Select hack.

odeke-em · 2018-05-03T06:02:13Z

I could start looking into the implementation of this, if there are no other takers...

@npat-efault all yours :) Thank you for offering to work on it!

odeke-em · 2018-11-13T09:06:58Z

How's it going @npat-efault? Kindly paging you as the Go1.12 cycle is a couple of weeks away from ending.

gopherbot · 2018-12-20T23:10:26Z

Change https://golang.org/cl/155517 mentions this issue: os: add SyscallConn method for os.File

robpike · 2018-12-20T23:18:13Z

I won't vote against this because I don't want a fight, but I wish it didn't go in to the os package as it encourages ugly I/O patterns that until now were only available through the syscall package, where they belong (if anywhere). Promoting this to the os package endorses them, and that's what I object to.

ianlancetaylor · 2018-12-20T23:40:56Z

Copying from the issue (should have replied here in the first place).

This is a pattern we already adopted for the net package. I don't see it quite as you do. The ugly I/O patterns still use the syscall package. I don't think we're really endorsing them. What we're doing here is permitting people to use the os package for normal use while dropping down to the syscall package when they have to, instead of saying that if they have one ugly syscall use then they have to use the syscall package for everything, and can never use the os package.

You're right of course that it is overly complicated but it's hard to just wish away the weird stuff that people want to do.

mikioh · 2018-12-21T01:38:05Z

@ianlancetaylor,

Can you please clarify what's the scope of the API? Does the API cover only a file, I mean, a byte-sequence, allowing partial reads/writes on a message that indicates an EOF-like signal, or more widely including networking stuff?

This is a pattern we already adopted for the net package.

Well, not exactly the same. The net package takes more information, e.g., a network parameter that indicates a set of communication capabilities, from API users to ensure correct behavior on work with the kernel, and provides more information for fault localization.

ianlancetaylor · 2018-12-21T03:44:17Z

@mikioh The API under discussion here already exists and is already used by the net package. It's https://golang.org/pkg/syscall/#Conn and https://golang.org/pkg/syscall/#RawConn. This issue is about implementing the same API in the os package, by adding a method to os.File that returns a syscall.Conn. You say that it's not exactly the same as what we do in the net package, but it is exactly the same.

zx2c4 · 2018-12-27T16:44:46Z

Very happy to see this land. This should improve things for TUN/TAP implementations quite a bit. Thanks a lot.

mikioh · 2018-12-28T02:29:29Z

@ianlancetaylor,

Thanks for the clarification. I see your comments "implementing the same API in the os package" and "exactly the same as what we do in the net package" and don't see a comment like "both package implementations are same." That means that os.File.SyscallConn and net.{TCP,UDP,IP,Unix}Conn.SyscallConn may return different error values and methods of syscall.RawConn may return different error values, depending on the package returning the syscall.RawConn.

ianlancetaylor · 2018-12-28T04:15:19Z

@mikioh That is true: the error values are not the same. It's not obvious to me that it makes sense for them to be the same, since the net package error values return an address, in the net.OpError.Source field, which does not exist for the os package error values.

cpuguy83 · 2019-01-08T16:41:17Z

I see this is in the 1.12 milestone. Is there a plan to have this in the next beta or pushed to 1.13?

ianlancetaylor · 2019-01-08T23:11:42Z

@cpuguy83 Yes, this is committed to tip and will be in the next beta release.

gopherbot · 2019-01-08T23:19:52Z

Change https://golang.org/cl/156839 mentions this issue: doc: go1.12: mention os.File.SyscallConn

Updates #24331 Change-Id: I2d7c996bbe29d5b3922588e199a106eb722c02e6 Reviewed-on: https://go-review.googlesource.com/c/156839 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

npat-efault changed the title ~~os: File.Fd() sets underlying filedes to non-blocking~~ os: File.Fd() sets underlying filedes to blocking Mar 11, 2018

andybons added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 11, 2018

andybons added this to the Go1.11 milestone Mar 11, 2018

fraenkel mentioned this issue Mar 21, 2018

net: goroutine leak when when calling File on a TCPConn #24455

Closed

WGH- mentioned this issue Mar 24, 2018

Linux inotify poller wastes an OS thread fsnotify/fsnotify#240

Closed

spf13 added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 26, 2018

gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 26, 2018

ianlancetaylor changed the title ~~os: File.Fd() sets underlying filedes to blocking~~ os: add File.SyscallConn method to permit frobbing file descriptor Jun 27, 2018

ianlancetaylor added the help wanted label Jun 27, 2018

ianlancetaylor modified the milestones: Go1.11, Go1.12 Jun 27, 2018

ianlancetaylor modified the milestones: Go1.12, Go1.13 Dec 20, 2018

crvv mentioned this issue Dec 20, 2018

proposal: net: add SyscallConn for minor protocols #28144

Closed

gopherbot closed this as completed in c0914d5 Dec 27, 2018

mikioh modified the milestones: Go1.13, Go1.12 Dec 28, 2018

cuonglm mentioned this issue Dec 28, 2018

Fix pending Read blocks after interface closed songgao/water#52

Closed

cretz mentioned this issue Dec 28, 2018

Support embedded control socket on Windows cretz/bine#23

Open

songgao mentioned this issue Jan 6, 2019

make sure *os.File is pollable so Close unblocks any pending Read songgao/water#53

Merged

gopherbot pushed a commit that referenced this issue Jan 8, 2019

doc: go1.12: mention os.File.SyscallConn

1d2e548

Updates #24331 Change-Id: I2d7c996bbe29d5b3922588e199a106eb722c02e6 Reviewed-on: https://go-review.googlesource.com/c/156839 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

This was referenced Jan 11, 2019

proposal: x/net/sctp: new package #22191

Open

syscall: allow registration of new socket types for package net #15021

Closed

golang locked and limited conversation to collaborators Jan 8, 2020

gopherbot added the FrozenDueToAge label Jan 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

os: add File.SyscallConn method to permit frobbing file descriptor #24331

os: add File.SyscallConn method to permit frobbing file descriptor #24331

npat-efault commented Mar 10, 2018

npat-efault commented Mar 10, 2018

npat-efault commented Mar 10, 2018

odeke-em commented Mar 11, 2018

as commented Mar 11, 2018

npat-efault commented Mar 11, 2018 •

edited

gopherbot commented Mar 11, 2018

npat-efault commented Mar 11, 2018

andybons commented Mar 11, 2018

rsc commented Mar 26, 2018

rsc commented Mar 26, 2018

npat-efault commented Apr 3, 2018 •

edited

zx2c4 commented Apr 19, 2018 •

edited

odeke-em commented May 3, 2018

odeke-em commented Nov 13, 2018

gopherbot commented Dec 20, 2018

robpike commented Dec 20, 2018

ianlancetaylor commented Dec 20, 2018

mikioh commented Dec 21, 2018

ianlancetaylor commented Dec 21, 2018

zx2c4 commented Dec 27, 2018

mikioh commented Dec 28, 2018 •

edited

ianlancetaylor commented Dec 28, 2018

cpuguy83 commented Jan 8, 2019

ianlancetaylor commented Jan 8, 2019

gopherbot commented Jan 8, 2019

os: add File.SyscallConn method to permit frobbing file descriptor #24331

os: add File.SyscallConn method to permit frobbing file descriptor #24331

Comments

npat-efault commented Mar 10, 2018

npat-efault commented Mar 10, 2018

npat-efault commented Mar 10, 2018

odeke-em commented Mar 11, 2018

as commented Mar 11, 2018

npat-efault commented Mar 11, 2018 • edited

gopherbot commented Mar 11, 2018

npat-efault commented Mar 11, 2018

andybons commented Mar 11, 2018

rsc commented Mar 26, 2018

rsc commented Mar 26, 2018

npat-efault commented Apr 3, 2018 • edited

zx2c4 commented Apr 19, 2018 • edited

odeke-em commented May 3, 2018

odeke-em commented Nov 13, 2018

gopherbot commented Dec 20, 2018

robpike commented Dec 20, 2018

ianlancetaylor commented Dec 20, 2018

mikioh commented Dec 21, 2018

ianlancetaylor commented Dec 21, 2018

zx2c4 commented Dec 27, 2018

mikioh commented Dec 28, 2018 • edited

ianlancetaylor commented Dec 28, 2018

cpuguy83 commented Jan 8, 2019

ianlancetaylor commented Jan 8, 2019

gopherbot commented Jan 8, 2019

npat-efault commented Mar 11, 2018 •

edited

npat-efault commented Apr 3, 2018 •

edited

zx2c4 commented Apr 19, 2018 •

edited

mikioh commented Dec 28, 2018 •

edited