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: add a function to construct *File with non-blocking file descriptor #22939

Closed
crvv opened this issue Nov 30, 2017 · 17 comments
Closed

os: add a function to construct *File with non-blocking file descriptor #22939

crvv opened this issue Nov 30, 2017 · 17 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Proposal Proposal-Accepted
Milestone

Comments

@crvv
Copy link
Contributor

crvv commented Nov 30, 2017

Change

Add a new function to construct a *File with a file descriptor, set non blocking and put it into runtime poller. A naive but working diff:

--- a/src/os/file_unix.go
+++ b/src/os/file_unix.go
@@ -74,14 +74,17 @@ func (f *File) Fd() uintptr {
 // NewFile returns a new File with the given file descriptor and
 // name. The returned value will be nil if fd is not a valid file
 // descriptor.
 func NewFile(fd uintptr, name string) *File {
        return newFile(fd, name, kindNewFile)
 }
 
+func NewFileNonBlocking(fd uintptr, name string) *File {
+       return newFile(fd, name, kindOpenFile)
+}

Benefit

With this, I can make use of the runtime poller and have SetDeadline method on custom network interfaces.
An example (on Linux):

package main

import (
	"os"
	"syscall"
	"time"
	"unsafe"
)

const IFF_TUN = 0x0001

func main() {
	f, err := syscall.Open("/dev/net/tun", os.O_RDWR, 0)
	if err != nil { panic(err) }
	fd := uintptr(f)

	var req ifreq
	req.flags = IFF_TUN
	copy(req.name[:], "tun0")
	ioctl(fd, syscall.TUNSETIFF, uintptr(unsafe.Pointer(&req)))

	conn := os.NewFileNonBlocking(fd, "tun0")

	packet := make([]byte, 2000)
	for {
		err := conn.SetDeadline(time.Now().Add(time.Second * 2))
		if err != nil { panic(err) }
		n, err := conn.Read(packet)
		if err != nil {
			println(err.Error())
		} else {
			println("read a packet with length", n)
		}
	}
}

func ioctl(fd uintptr, request uintptr, ifreq uintptr) {
	_, _, errno := syscall.Syscall(syscall.SYS_IOCTL, fd, request, ifreq)
	if errno != 0 { panic(errno) }
}

type ifreq struct {
	name  [16]byte
	flags uint16
}

Related issue

#15021
It looks like the main motivations of #15021 are runtime poller and SetDeadline.
Both of them can be solved by this very simple function.

@gopherbot gopherbot added this to the Proposal milestone Nov 30, 2017
@ianlancetaylor
Copy link
Contributor

Perhaps a better approach would be to add a mechanism for the os package to use syscall.RawConn, much as we've recently added to the net package. That would let you use os.OpenFile and then call the ioctl you need.

@crvv
Copy link
Contributor Author

crvv commented Dec 1, 2017

That would let you use os.OpenFile and then call the ioctl you need.

I figured out that the tun fd must be put into epoll after the ioctl call, otherwise epoll doesn't work on it. But os.OpenFile will do the epoll things before ioctl.

Besides, the fd can also come from other syscalls like socket.

@ianlancetaylor
Copy link
Contributor

If the fd comes from socket, then it ought to use functions in the net package.

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

@crvv
Copy link
Contributor Author

crvv commented Dec 1, 2017

To achieve the goal (make use of runtime poller from outside the stdlib), the net package ways are #15021 and #10565. They are much more complex.
I just think it is a very simple solution that put a file descriptor into runtime poller through os package.

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

I think this is a better solution.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2017

We should fix this one way or another. Certainly returning EAGAIN from File.Read is not useful to clients, since they don't know when is again.

@rsc rsc added Proposal-Accepted NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Dec 4, 2017
@rsc rsc changed the title proposal: os: add a function to construct *File with non-blocking file descriptor os: add a function to construct *File with non-blocking file descriptor Dec 4, 2017
@rsc
Copy link
Contributor

rsc commented Dec 4, 2017

Leaving for @ianlancetaylor to decide the exact fix.

@odeke-em
Copy link
Member

@ianlancetaylor, I am going to move this out of the "Proposal" milestone and into Go1.11 since it was accepted. Please feel free to revert.

@gopherbot
Copy link

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

@npat-efault
Copy link
Contributor

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

@ianlancetaylor
Copy link
Contributor

Regarding CL 100077, I'm concerned that some users may set up their own nonblocking files, pass them to os.NewFile, and plan to handle EAGAIN errors themselves. For example, this seems to be the real problem behind #20915. It's an odd use case, but since it works today we should consider whether it is OK to break it.

@ianlancetaylor
Copy link
Contributor

It now occurs to me that programs that want to use the os package with nonblocking files can still do so after CL 100077 by starting the files in blocking mode, calling os.NewFile, and then calling syscall.SetNonblock.

@riobard
Copy link

riobard commented Mar 23, 2018

Will calling syscall.SetNonblock correctly register the fd to runtime poller?

@ianlancetaylor
Copy link
Contributor

No, but the case I'm concerned about with regard to Cl 100077 is people who want to use a non-blocking FD. Their code can work if they pass a blocking FD to os.NewFile and then call syscall.SetNonblock to switch the file into non-blocking mode.

@kirbyzhou
Copy link

if Stdin and Stdout is Linux FIFO/Pipe, can I use any method above to avoid blocking deadlock?

@ianlancetaylor
Copy link
Contributor

@kirbyzhou No, sorry, this does not apply to stdin/stdout, and those not going to be in non-blocking mode anyhow. In any case everything will work with stdin/stdout, you just won't be able to call SetDeadline.

@kirbyzhou
Copy link

kirbyzhou commented Apr 16, 2018

So if I want to do Graceful Exit with I/O blocked on Stdon/Stdout which is FIFO/PIPE, what can I do?

For example, the program read plain text from Stdin, and write compressed data to Stdout.
When ctrl-C / kill happens, I want the program to write a valid file end before exit.

@ianlancetaylor
Copy link
Contributor

It's not clear to me what ^C has to do with non-blocking standard input. If you want to handling ^C, use signal.Notify from the os/signal package.

In Go it is always easy to start a goroutine to handle something else. You can always set a deadline by starting a new goroutine or by using time.AfterFunc. Being able to call SetDeadline improves efficiency significantly when using hundreds of file descriptors. For just standard input, efficiency is unimportant, and the language provides multiple mechanisms for handling deadlines.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Proposal Proposal-Accepted
Projects
None yet
Development

No branches or pull requests

8 participants