go: io: Copy is easy to misuse and leak goroutines blocked on reads

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

All

Does this issue reproduce with the latest release?

Yes

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

N/A

What did you do?

Because of the way io.Copy works (and really any other solution to the same need) it is very easy to wedge a goroutine, and probably an fd along with it.

Example:

pr1, _ := io.Pipe()
_, pw2 := io.Pipe()
go io.Copy(pw2, pr1)

pw2.Close()

In this case, the goroutine will leak once pw2 is closed because there is no way to cancel that copy without also closing pr1. In some cases this makes sense, especially if you control both pipes.

In docker/moby this issue is a common case where we have a client connected over a unix or TCP socket, it is attached to a container’s stdio streams. If the client disconnects there is no mechanism (aside from platform specific code like epoll) to get notified that the copy operation needs to be cancelled.

I suggest an interface be added, potentially to io to receive close notifications from the runtime. Example:

type CloseNotifier() {
    CloseNotify() <-chan struct{}
}

The actual interface could be something like (maybe something that returns a context that gets cancelled?). I’m not married to the above interface (I know there’s a similarly named one in net/http).

*os.File and net.Conn implementations could implement this interface to help with the above scenario, and since the go runtime is already doing a bunch of fd notification magic it should generally be easily supported in the stdlib across platforms.

About this issue

Original URL
State: open
Created a year ago
Comments: 16 (6 by maintainers)

Most upvoted comments

I don’t understand what would be reading from that channel. How does it help with the io.Copy, which is presumably sitting in a call to Read?

ianlancetaylor on Feb 21, 2023

@rittneje On Linux I would expect this to be waiting for EPOLLHUP, however half-closed (EPOLLRDHUP) is indeed another scenario that should definitely be considered when designing the interface.

I don’t think it’s particularly useful to say io.Copy is not safe. It is for the most part pretty straight forward and even has performance optimizations (with support for splice, sendfile, and copy_file_range).

cpuguy83 on Feb 27, 2023

@bcmills We do use the read side. Your example also doesn’t work if/when the client does a close-write on the socket, which wouldn’t be completely unheard of it the client has nothing to send and is only expecting to receive.

cpuguy83 on Feb 22, 2023

If the client disconnects there is no mechanism (aside from platform specific code like epoll) to get notified that the copy operation needs to be cancelled.

.Read returns io.EOF (or an other error if the connection was killed or a deadline is reached) on all OSes, you don’t need to implement anything using epoll the runtime do this for you already. See here this very simple example:

package main

import (
	"fmt"
	"io"
	"net"
	"os"
)

func main() {
	conn, err := net.Dial("tcp", "localhost:11111")
	if err != nil {
		panic(err)
	}
	defer conn.Close()

	_, err = io.Copy(os.Stdout, conn)
	fmt.Println("copy exited:", err)
}

If I listen using nc -lvp 11111, run go run a.go and finaly CTRL + C netcat, I see:

copy exited: <nil>

I think you have some other bug and this is not related to io.Copy (timeouts ?).

*os.File and net.Conn implementations could implement this interface to help with the above scenario, and since the go runtime is already doing a bunch of fd notification magic it should generally be easily supported in the stdlib across platforms.

They already do by returning an error on .Read when the ressource is closed while you were reading from it.

Jorropo on Feb 22, 2023