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.

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

bufio: make zero value of Reader useful #45374

Closed
dsnet opened this issue Apr 3, 2021 · 8 comments
Closed

bufio: make zero value of Reader useful #45374

dsnet opened this issue Apr 3, 2021 · 8 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@dsnet
Copy link
Member

dsnet commented Apr 3, 2021

Consider the following code pattern:

var br bufio.Reader
for _, r := range readers {
    br.Reset(r)
    ... // make use of br
}

Currently, this pattern results in an infinite loop since the internal buffer of bufio.Reader is empty (resulting in no progress being made). The bufio.Reader.Reset method should initialize the internal buffer with defaultBufSize if it is empty.

@ghost
Copy link

ghost commented Apr 4, 2021

this pattern results in an infinite loop

How so? The program below doesn't hang.

package main

import (
	"bufio"
	"io"
	"os"
	"strings"
)

func main() {
	var br bufio.Reader
	readers := []io.Reader{strings.NewReader("data\n")}
	for _, r := range readers {
		br.Reset(r)
		io.Copy(os.Stdout, &br)
	}
}

@dsnet
Copy link
Member Author

dsnet commented Apr 4, 2021

Interesting, this program does hang:

package main

import (
	"bufio"
	"io"
	"strings"
)

func main() {
	var br bufio.Reader
	readers := []io.Reader{strings.NewReader("data\n")}
	for _, r := range readers {
		br.Reset(r)
		br.ReadBytes('\n')
	}
}

@ahmetakturk
Copy link
Contributor

ahmetakturk commented Apr 4, 2021

this pattern results in an infinite loop

How so? The program below doesn't hang.

It does not hang, when calling io.Copy. Because io.Copy calls src.WriteTo, bufio.Reader.WriteTo here, and in turn, bufio.Reader.WriteTo calls the internal reader's (strings.Reader here) WriteTo function.

@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 5, 2021
@ahmetakturk
Copy link
Contributor

Isn't it OK just doing this:

func (b *Reader) Reset(r io.Reader) {
        // add this if statement
	if len(b.buf) < minReadBufferSize {
		b.buf = make([]byte, minReadBufferSize)
	}
	b.reset(b.buf, r)
}

@yunnian
Copy link

yunnian commented Apr 7, 2021

Isn't it OK just doing this:

func (b *Reader) Reset(r io.Reader) {
        // add this if statement
	if len(b.buf) < minReadBufferSize {
		b.buf = make([]byte, minReadBufferSize)
	}
	b.reset(b.buf, r)
}

This will not save memory

@dsnet
Copy link
Member Author

dsnet commented Apr 7, 2021

Isn't it OK just doing this:

Yea, that's more or less what's proposed with saying "bufio.Reader.Reset method should initialize the internal buffer with defaultBufSize if it is empty."

This will not save memory

I'm not sure what you're getting at. This issue isn't about saving memory, but making the zero value of bufio.Reader useful.

@gopherbot
Copy link

Change https://golang.org/cl/307991 mentions this issue: bufio : Solve the problem of infinite loop after reset

@ianlancetaylor ianlancetaylor added this to the Backlog milestone Apr 19, 2021
@gopherbot
Copy link

Change https://golang.org/cl/345570 mentions this issue: bufio: make Reader.Reset and Writer.Reset work on the zero value

@golang golang locked and limited conversation to collaborators Sep 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

7 participants
@ianlancetaylor @ALTree @yunnian @dsnet @gopherbot @ahmetakturk and others