Thank you for raising this issue.

Unlike many projects, the Go project does not use GitHub Issues for general discussion or asking questions. GitHub Issues are used for tracking bugs and proposals only.

For asking questions, see:

• Stack Overflow with questions tagged "go"

• The Go Forum, a web-based forum

• Gophers Slack, use the invite app for access. The #general channel is a good starting point.

• Go Community on Hashnode with questions and posts tagged with "go"

• The golang-nuts mailing list

• IRC channel #go-nuts on Freenode

@shenmingxing please post a minimized program that reproduces the error and the full stack trace as text (not image).

I've run into an identical crash, this time with a bufio reading from rand.Reader on linux (5.4). Working on figuring out a way to reproduce it, but it appears that the bug stems from the Read call returning a number larger than the input buffer. So the bug would be partially that bufio doesn't check that n is within a valid range.

@evanphx if you can produce a code sample that would be extremely valuable.

@davecheney Yeah, I'm going to work on that the next couple of days. There are 2 vectors to reproduce it with, at least for me. So I should have something for ya.

@davecheney I figured it out. It’s not a bug in bufio. This happens when a bufio is used concurrently by many goroutines. When the refill code runs concurrently, the b.w += n line pushes the w to 8192 as we’ve seen here.

So basically, folks accidentally use a bufio.Reader across goroutines, probably because we get used to os.File working across goroutines and therefore not being careful enough with io.Reader values.

Try running the race detector which should have found that issue. https://golang.org/doc/articles/race_detector.html

If it is not found by the race detector please file a bug with a reproducer program where the race detector doesnt find the race.

It's hard to get the race detection to hit this exact case because it fires on pretty much all parts of bufio#Read.

@evanphx could you explain what you mean? Are you saying the race detector fires on all uses of bufio.Read ?

@davecheney I'm saying that if you spin up calls to bufio#Read from many goroutines, the race detector fires all over it, every read/write of b.r and b.w. For the very specific case of this bug, I need it to fire on b.w += n but it's firing in same many other places it hasn't fired on b.w += n. Let me run it for like a minute or something, the case of b.w += n is on the refill case which is just much more rare than the other uses.

@davecheney I'm saying that if you spin up calls to bufio#Read from many goroutines, the race detector fires all over it, every read/write of b.r and b.w. For the very specific case of this bug, I need it to fire on b.w += n but it's firing in same many other places it hasn't fired on b.w += n. Let me run it for like a minute or something, the case of b.w += n is on the refill case which is just much more rare than the other uses.

That is expected, that type is not safe for multiple concurrent access.

Yup! Very expected. The situation this bug shows up on is this line: https://github.com/golang/go/blob/master/src/bufio/bufio.go#L234 When 2 goroutines get to line 227, stall, and then both end up getting to 234 and incrementing b.w. As requested by @martisch I was just trying to validate that the race detector would catch up the race on coming from line 234.

I was able to reproduce the exact crash. The race detector did not report it though, I think because it tracks a read from a write by another goroutine, but this particular crash can be induced by goroutine 1 doing b.w += n, then goroutine 2 doing b.w += n, then goroutine 2 trying to to do a Read and crashing. This specific set of events would not fire the race detector but it would cause a crash.

Here is the reproduction:

package pb

import (
	"bufio"
	"context"
	io "io"
	"testing"
	"time"
)

type slowReader struct {
}

func (_ slowReader) Read(b []byte) (int, error) {
	time.Sleep(time.Second)
	return len(b), nil
}

func TestULID(t *testing.T) {
	ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
	defer cancel()

	var sr slowReader

	r := bufio.NewReaderSize(sr, 128)

	for i := 0; i < 100; i++ {
		go func() {

			buf := make([]byte, 126)
			for {
				select {
				case <-ctx.Done():
					return
				default:
				}

				io.ReadFull(r, buf)
			}
		}()
	}

	<-ctx.Done()
}

It is ok if the race detector fires on some part of the code. It doesn't need to be the specific line. My thinking was that if the race detector is able to detect the code (not specific) line isn't race free then its an easy way to check to take action and make the program around the Reader race free before filing a bug for the Reader code.