proposal: bufio: add a new method "Handover" for Writer and Reader

[panjf2000]

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

$ go version
go version go1.16.5 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN="/data/go/bin"
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/data/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/data/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/snap/go/7736"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/snap/go/7736/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.5"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1082148225=/tmp/go-build -gno-record-gcc-switches"

What did you do?

bufio.Writer:

type Writer struct {
	err error
	buf []byte
	n   int
	wr  io.Writer
}

I set up a net.Conn c as the underlying io.Writer wr of bufio.Writer bw and replaced bw.wr with a bytes.Buffer bb (by calling bufio.Reset(bb)) when the TCP socket disconnected caused by any network problem, so other goroutines can continue calling bufio.Write()/Flush() to write/flush data into the local buffer bb (bw.wr), not knowing the network problem, I will create a new net.Conn and replace bw.wr with it, then copy all data in bb to bw.buf once the TCP socket resumes.

Because the bw was shared by multiple goroutines (goroutine-safe along with mutex), some of goroutines might have called bufio.Write() writing data into the bw.buf before bufio.Reset(bb) was called, which means those data will be lost, when I reconnected to the remote server, I want to send all buffered data (including data was in bw.buf before bufio.Reset(bb) was called) to the remote server.

What did you expect to see?

All buffered data was sent to the remote server after a reconnection.

What did you see instead?

I lost some data by calling bufio.Reset(bb).

So I propose to add a new method "Handover" for Writer and Reader, Handover is the same as Reset, except it retains all unflushed buffered data and migrates them to the new underlying io.Writer.

It would be something like this:

func (b *Writer) Handover(w io.Writer) error {
	if err := b.err; err != nil || b.n == 0 {
		b.Reset(w)
		return err
	}
	b.wr = w
	b.err = b.Flush()
	return b.err
}

func (b *Reader) Handover(r io.Reader) error {
	n := b.Buffered()
	if err := b.err; err != nil || n == 0 {
		b.Reset(r)
		return err
	}
	b.rd = r
	b.err = nil
	return nil
}

I can call bw.Handover(bb) to retain all unflushed buffered data and flush it to bytes.Buffer bb instead of calling bw.Reset(bb) to discard all unflushed buffered data in bw.buf.

[davecheney]

What you are asking for is not race safe.

[davecheney]

If you want to implement this logic, insert your own type between bufio and net Conn. Then you have the hooks to reopen network connections as you desire.

[panjf2000]

If you want to implement this logic, insert your own type between bufio and net Conn. Then you have the hooks to reopen network connections as you desire.

That results in one extra data copy which I'm trying to avoid.

[davecheney]

Btw, I don’t think this will be reliable because data may be lost between the sender and receiver in such a way that you cannot correctly determine the number of bytes received so you can correctly rewind the writer. This problem of distributed coordination is considered non trivial.

[davecheney]

If you want to implement this logic, insert your own type between bufio and net Conn. Then you have the hooks to reopen network connections as you desire.

That results in one extra data copy which I'm trying to avoid.

I don’t believe this is correct. io.Writer does not require intermediaries to buffer data.

[panjf2000]

If you want to implement this logic, insert your own type between bufio and net Conn. Then you have the hooks to reopen network connections as you desire.

That results in one extra data copy which I'm trying to avoid.

I don’t believe this is correct. io.Writer does not require intermediaries to buffer data.

The thing is, there is already some unflushed buffered data in bw.buf, how can I migrate it to a new io.Writer like bytes.Writer?

[davecheney]

I don’t think think you need to module bufio to do what you’re asking. I also think what you are asking is unsound because tcp will lie to you. I think you can prove this to yourself, or prove me wrong, with an straight net.Conn. If you can build a wrapper type that can handle the underlying net.Conn going away and being reconnected, then wrapping a bufio Writer around that whole thing for convenience can be done afterwards.

[panjf2000]

Let's just put the TCP aside, I still think that bufio.Writer outght to allow users to retain buffered data when switching the underlying io.Writer, for some scenarios.

Discarding all buffered data is also unsound to me from where I stand.

[davecheney]

Maybe, but why should the ability to switch underlying writers be restricted to just bufio? That feels like the domain of a different type which itself implements io.writer

[panjf2000]

In case you didn't notice, actually bufio.Writer itself is already a wrapper of io.Writer, thus it's rational to ask it to evolve more abilities.

[panjf2000]

It just really bothers me that bufio allows users to switch the underlying io.Writer of bufio.Writer (by bufio.Reset(w)) and yet forbids them to do so while retaining unflushed buffered data.

[ianlancetaylor]

I agree with @davecheney : write your own type that does what you need. There is nothing particularly special about bufio.Writer. What you are looking for seems very special purpose.

[panjf2000]

My case might be specific, but it should be all-purpose for a new method that is like Reset() but retains buffered data. I only brought my case out for illustrating the requirement and I believe there are more scenarios.