bugfix:save file.Fd() to avoid block again

[detailyang]

See the code below:

1. setNonblock(desc.fd(), true)
2. desc.fd()

The second desc.fd() will set the kernel file description block again.

#11 is likely related.

[detailyang]

@rvasily

Would you have a look at this?

Many thanks:)

[gobwas]

Hi! Could you please follow this rules for comments? Thanks!

[detailyang]

fixed.

[gobwas]

Why this now must be exported?

[detailyang]

since I need the underlying fd in my scene so I have to export the fd:(.

[detailyang]

In my scene, I need the fd and join the fd to another epoll fd which is super strange but it works. I use easygo to solve golang/go#15735

[gobwas]

Is it really necessary to provide additional Desc constructor here? Why just don't setup fd field of the struct right in place where it is created and remove whole Fd() getter method?

[gobwas]

@rvasily looks like this PR is better than this.

[rdmrcv]

Might be useful to take a look at the syscall.RawConn (https://golang.org/pkg/syscall/#RawConn). Since go 1.9 it may allow to avoid FD duplication at all.

[detailyang]

@gobwas sorry, I'm busy:) can you look at this.

@ligser RawConn says the following:

  // The file descriptor fd is guaranteed to remain valid while
      // f executes but not after f returns.

The fd is invalid after f returns although we know the fd should never be changed?

[rdmrcv]

@detailyang Yes, I think about that notice too, but in the stdlib code it does not looks like the FD changed anywhere. So I think this notice about the FD state not the FD value.

But anyway if you create a new method like:

func (h *Desc) Control(f func(fd uintptr) error) (err error) {
	// Here we rely that func will be executed in the same goroutine and we can
	// receive new err value at the end (err captured to the func).
	cntErr := h.rawConn.Control(func(fd uintptr) {
		err = f(fd)
	})
	runtime.KeepAlive(h)
	if cntErr != nil {
		return cntErr
	}

	return err
}

and store a syscall.RawConn you can do not worry about it. When you need a FD you pass a callback and work with the valid FD inside it.

[rdmrcv]

But that approach does not widely tested. I use it in my own implementation and do not know about caveats. Maybe repo owners see something that I missed about that approach.

[detailyang]

@ligser many thanks.

I do not understand the way you use runtime.Keepalive to keepalive h don't GC? it looks makes no sense because the h will not gc in the method of h.

I check the code in https://github.com/golang/go/blob/master/src/internal/poll/fd_unix.go#L512 which I think it says the fd must be associated with the opened file but after the callback return the fd maybe is associated with the closed file or another file which the new fd is the value..

[rdmrcv]

@detailyang https://groups.google.com/forum/#!topic/Golang-nuts/LIWj6Gl--es there is some doubts about method arguments and GC. Anyway KeepAlive here just for ensure and just because I see same pattern somewhere in stdlib it may be not required.

[rdmrcv]

Oh, looks like I seen that in net/rawconn, and looks like the KeepAlive should not be required in my snippet.