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.

Sign up for GitHub

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

Jump to bottom

net: reading a connectionless datagram socket after a CloseRead still blocks indefinitely #15250

Closed
alexcesaro opened this issue Apr 12, 2016 · 10 comments
Closed

net: reading a connectionless datagram socket after a CloseRead still blocks indefinitely #15250

alexcesaro opened this issue Apr 12, 2016 · 10 comments
Labels
FrozenDueToAge

Comments

@alexcesaro
Copy link
Contributor

I was trying to cleanly close a Unix socket (i.e. stop receiving traffic, read all the socket buffer and then close the socket) but found it a bit cumbersome.

The issue is that the following code blocks indefinitely:

package main

import (
    "fmt"
    "net"
)

func main() {
    var socket = "/tmp/test.sock"
    addr, err := net.ResolveUnixAddr("unixgram", socket)
    if err != nil {
        panic(err)
    }

    conn, err := net.ListenUnixgram("unixgram", addr)
    if err != nil {
        panic(err)
    }

    w, err := net.DialUnix("unixgram", nil, conn.LocalAddr().(*net.UnixAddr))
    if err != nil {
        panic(err)
    }
    if _, err := w.Write([]byte("foo")); err != nil {
        panic(err)
    }

    if err := conn.CloseRead(); err != nil {
        panic(err)
    }

    buf := make([]byte, 10)
    n, err := conn.Read(buf)
    if err != nil {
        panic(err)
    }
    fmt.Println(string(buf[:n])) // Prints foo.

    n, err = conn.Read(buf) // Hangs indefinitely. Should it return io.EOF?
    if err != nil {
        panic(err)
    }
    fmt.Println(string(buf[:n]))
}

To read all the socket buffer without blocking indefinitely you have to either:

  1. use conn.Close() after a timeout to unblock conn.Read()
  2. use conn.File() and syscall.SetNonblock() to read the connection buffer in a non-blocking way

I don't like 1. since it would make my tests slow or flaky. 2. works well but it makes the code more complex.

Since after a CloseRead(), a socket cannot receive more data I think it would make sense that Read returns an io.EOF when it reaches the end of the connection buffer.
@mikioh
Copy link
Contributor

mikioh commented Apr 12, 2016

Which operating system/version are you running?

@alexcesaro
Copy link
Contributor Author

Go 1.6 on Ubuntu.

@mikioh
Copy link
Contributor

mikioh commented Apr 12, 2016

CloseRead and CloseWrite call shutdown system call internally. I guess that the functionality is just for full-duplex connection-oriented sockets such as "tcp" or "unix". I suspect that Linux kernel doesn't support shutdown on connectionless datagram sockets such as "udp" or "unixgram".

@alexcesaro
Copy link
Contributor Author

shutdown works fine with unixgram, I haven't tried with UDP but I guess it would work too.

To be clear, everything works fine with CloseRead. I just think the behavior of Read should be updated to return io.EOF when CloseRead has been called and the socket buffer is empty.

@mikioh
Copy link
Contributor

mikioh commented Apr 12, 2016

shutdown works fine with unixgram

Good to know, Linux always keeps moving forward, thanks. Though, I'm not sure what's the best way to make consistent behavior across all supported platforms.

@alexcesaro
Copy link
Contributor Author

I am not very familiar with the internals of the net package and I am probably doing it wrong but here is the kind of fix I was thinking of: https://go-review.googlesource.com/#/c/21871/

I haven't looked on implementing it on the other platforms but some questions need to be answered first:

  • Is it ok to update Read behavior in this case?
  • Is it acceptable to add a new field to netFD just for this use case?
  • If not, is there a smarter way to do it?

@mikioh
Copy link
Contributor

mikioh commented Apr 13, 2016

@alexcesaro,

Before having a look at your code, can you please make sure your proposal about the following:

  • CloseRead method; behavior of shutdown on connectionless datagram sockets
    • just to put a CANNOT-READ-ANYMORE mark onto the socket, or have a mechanism for avoiding to make a blocked drain?
    • if the former, what's the plan to prevent the blocked drain between socket and transport protocol layers? in general, a far-end socket never knows you stops reading on connectionless datagram sockets (because of the lack of control plane for such stuff)
  • CloseWrite method; behavior of shutdown on connectionless datagram sockets
    • what's your proposal?
  • Read method; meaning of io.EOF on connectionless datagram sockets
    • it represents... end of what?

@mikioh mikioh changed the title net: reading a socket buffer after a CloseRead still blocks indefinitely net: reading a connectionless datagram socket after a CloseRead still blocks indefinitely Apr 13, 2016
@alexcesaro
Copy link
Contributor Author

Sorry I haven't been clear enough: I am only speaking of datagrams on Unix sockets and not UDP.

Currently the *UnixConn.CloseRead method already exists. When it is used on an "unixgram" socket, writes to the Unix socket fail but it is still possible to read the packets in the buffer of the socket. And I don't want to change any of these behaviors.

So to answer your questions:

CloseRead method; behavior of shutdown on connectionless datagram sockets

I don't want to change the current behavior.

CloseWrite method; behavior of shutdown on connectionless datagram sockets

I don't want to change the current behavior.

Read method; meaning of io.EOF on connectionless datagram sockets: it represents... end of what?

It represents the end of incoming data since the socket cannot be written anymore.

@mikioh
Copy link
Contributor

mikioh commented Apr 14, 2016

@alexcesaro,

I'm still confused. When I run https://github.com/mikioh/-stdyng/tree/master/cmd/ugrmshtdwn, which is based on your snippet, I have a few different results like the following:

Linux 4.4.x:
#0: 3 bytes transferred from @ to /tmp/ugrmshtdwn.sock
#0: 3 bytes transferred to /tmp/ugrmshtdwn.sock from <nil>
#1: 3 bytes transferred from @ to /tmp/ugrmshtdwn.sock
#1: 3 bytes transferred to /tmp/ugrmshtdwn.sock from <nil>
one more: read unixgram /tmp/ugrmshtdwn.sock: i/o timeout

OS X 10.11.x (and some BSDs maybe?):
#0: 3 bytes transferred from  to /tmp/ugrmshtdwn.sock
#0: 3 bytes transferred to /tmp/ugrmshtdwn.sock from <nil>
#1: 3 bytes transferred from  to /tmp/ugrmshtdwn.sock
close unixgram /tmp/ugrmshtdwn.sock: shutdown: socket is not connected
#1: 3 bytes transferred to /tmp/ugrmshtdwn.sock from <nil>
one more: read unixgram /tmp/ugrmshtdwn.sock: i/o timeout

Other BSDs:
#0: 3 bytes transferred from  to /tmp/ugrmshtdwn.sock
#0: 3 bytes transferred to /tmp/ugrmshtdwn.sock from <nil>
#1: 3 bytes transferred from  to /tmp/ugrmshtdwn.sock
#1: 0 bytes transferred to /tmp/ugrmshtdwn.sock from <nil>
one more: 0 bytes transferred to /tmp/ugrmshtdwn.sock from <nil>

Probably the results depend on each implementation, especially the plumbing between socket layer that manages the general connection-oriented/connectionless state, and transport protocol layer that is the real conveyer.

I suppose that if you really want to modify the behavior of Read, you also need to modify the behavior of CloseRead for avoiding confusion and incnsistency. I'm still not sure what we should do. At least we need to clarify three things before moving forward; 1) the desired behavior of CloseRead on unixgram socket, 2) the desired behavior of Read on cant-read-anymore unixgram socket, 3) whether we should use io.EOF as a cannot-read-anymore signal on unixgram socket, and which method should do.

PS: Please change the description appropriately, like "proposal: net: blah blah".

@alexcesaro
Copy link
Contributor Author

Thanks that's very interesting.

I wrongly assumed that shutdown was working with unxigrams but after having a look at the doc of shutdown, I see that it only mentions full-duplex connections. So using it on unixgram sockets is an unspecified behavior.
I was relying on an unspecified behavior and should have been using "unix" or "unixpacket" instead.

I think it is a very bad idea to try to unify an unspecified behavior on multiple platforms. So I am closing this issue. Thanks for your patience!

@golang golang locked and limited conversation to collaborators Apr 14, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mikioh @alexcesaro @gopherbot