Skip to content

net: File method of {TCP,UDP,IP,Unix}Conn and {TCP,Unix}Listener should leave the socket in nonblocking mode #24942

Closed
@rothskeller

Description

@rothskeller

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

1.10.1

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

linux/amd64 and darwin/amd64

What did you do?

listener, _ := net.Listen("tcp", ":8000")
listenerFH, _ := listener.(*net.TCPListener).File()
server.Serve(listener)
// later, in a different goroutine
server.Shutdown(context.Background())

What did you expect to see?

Server graceful shutdown.

What did you see instead?

Shutdown() returned immediately but Serve() never returned.

Diagnosis

The call to File() on the listener puts the socket in blocking mode. As a result, when the listener is closed, the Accept() called by Serve() does not return. Note that, if the call to File() is moved so that it occurs after the call to Accept(), the problem does not occur.

Response from iant@golang.org on golang-nuts, April 18, 2018:

Before about Go 1.9 or so the os package could not handle a non-blocking descriptor, so File had to change the connection to blocking mode in order to make it usable. Now, however, the os package does use nonblocking descriptors where possible. We could probably change the File method to use a nonblocking *os.File. Please do open an issue for that.

Activity

rothskeller

rothskeller commented on Apr 19, 2018

@rothskeller
Author

Probably irrelevant detail: the reason I am calling File() on the listener at all is so that I can add it to (exec.Command).ExtraFiles to pass it to a subprocess. Definitely no desire to change the socket behavior in so doing.

changed the title [-](*net.TCPListener).File() should leave the socket in nonblocking mode[/-] [+]net: (*net.TCPListener).File() should leave the socket in nonblocking mode[/+] on Apr 19, 2018
added this to the Go1.11 milestone on Apr 19, 2018
fraenkel

fraenkel commented on Apr 19, 2018

@fraenkel
Contributor

@ianlancetaylor The change is trivial but do we care about the behavior change? Unlike what was done with os.File, we cannot easily enhance an existing method.

ianlancetaylor

ianlancetaylor commented on Apr 19, 2018

@ianlancetaylor
Contributor

The package net methods return a *os.File, which will continue to work as expected if we make it nonblocking. I think it would be hard to write a program to detect this change. Let me know if you think otherwise.

fraenkel

fraenkel commented on Apr 19, 2018

@fraenkel
Contributor

The only nuance I am aware is that Deadline calls now work.

fraenkel

fraenkel commented on Apr 19, 2018

@fraenkel
Contributor

Shall this also apply to IPConn and TCPConn or just TCPListener? Seems weird to only have one behave differently, plus its easier to affect all 3.

ianlancetaylor

ianlancetaylor commented on Apr 19, 2018

@ianlancetaylor
Contributor

I assume it should affect all three.

gopherbot

gopherbot commented on Apr 19, 2018

@gopherbot
Contributor

Change https://golang.org/cl/108297 mentions this issue: net: calling File leaves the socket in nonblocking mode

twmb

twmb commented on Apr 19, 2018

@twmb
Contributor

I would guess that fd's were set to nonblocking to begin with (a144e3e) was for a roundabout way of deregistering from epoll (the logic is a bit to follow for the repo at that time). dup would've been problematic because a Close did not deregister from epoll, meaning a registered fd would never be unregistered.

netFD's now unconditionally fall into EPOLL_CTL_DEL on close, so this is no longer an issue. Removing SetNonblocking from netFD narrows #24481 down to the os package specifically. This might fix #24455 (but not the real underlying issue, if any).

twmb

twmb commented on Apr 19, 2018

@twmb
Contributor

Didn't realize this issue had closed (was looking into commits for too long with this page open) -- I'll move portions of that last comment to the relevant open issues.

mikioh

mikioh commented on Apr 20, 2018

@mikioh
Contributor

Note that this might be a surprise to people who rely on the go1compt promise. What happens when people having two executables built by go1.10 or below and go1.11 or above, and the executables need to share a single underlying socket? The go1.10 executable assumes that the passed cloned descriptor points to the blocking mode socket but the go1.11 executable passes the cloned descriptor pointing to the non-blocking mode socket.

Of course, there's no problem when people use net.File{Conn,PacketConn,Listener} functions in the go1.10 executables instead of functions from the syscall and x/net/unix packages. Is it safe to bet/assume on that? (not sure, probably it's okay because functions and primitives provided by syscall and x/net/unix packages are just for people who understand what's the right thing and what they are doing)

ianlancetaylor

ianlancetaylor commented on Apr 20, 2018

@ianlancetaylor
Contributor

@mikioh I think that the cases you are describing are fairly unlikely. I agree that there are cases that can break. Let's see what happens. If people find bugs, we can roll back.

changed the title [-]net: (*net.TCPListener).File() should leave the socket in nonblocking mode[/-] [+]net: File method of {TCP,UDP,IP,Unix}Conn and {TCP,Unix}Listener should leave the socket in nonblocking mode[/+] on Apr 20, 2018
locked and limited conversation to collaborators on Apr 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @mikioh@fraenkel@twmb@rothskeller@ianlancetaylor

        Issue actions

          net: File method of {TCP,UDP,IP,Unix}Conn and {TCP,Unix}Listener should leave the socket in nonblocking mode · Issue #24942 · golang/go