using bytes.Buffer cross goroutines

[divinerapier]

Hi,
This pr using bytes.Buffer cross goroutines which is not goroutines safety.

I did a testing:

package main

func main() {
	var ok int
	buffer := new(bytes.Buffer)
	go func() {
		for i := 0; i < 10; i++ {
			buffer.WriteString(strconv.FormatInt(int64(i), 10))
		}
	}()


	for ok < 10 {
		var buf [1]byte
		n, err := io.ReadFull(buffer, buf[:])
		if err != nil || n != len(buf) {
			fmt.Printf("READ ERR. i: %d. n: %d. error: %v\n", ok, n, err)
			continue
		}
		if ok != int(buf[0]-'0') {
			fmt.Printf("ERROR: index: %d. b: %v\n", ok, buf[0])
		} else {
			fmt.Printf("OK: index: %d. b: %v\n", ok, buf[0])
		}
		ok++
	}
}

Run it with -race option should be panic.

go run -race .

Thanks.

[cyphar]

As far as I can tell there is no race -- we serialise access to the buffer through errChan (the main thread waits for an error through errChan before reading -- and that only happens after the goroutine is finished writing). Your example code doesn't match what we're actually doing in runc. Using your example code, but modified to match what the code in runc, go run -race doesn't produce any panics:

package main

import (
	"bytes"
	"fmt"
	"io"
	"strconv"
)

func main() {
	var ok int
	buffer := new(bytes.Buffer)
	errChan := make(chan error, 1) // <--
	go func() {
		for i := 0; i < 10; i++ {
			buffer.WriteString(strconv.FormatInt(int64(i), 10))
		}
		errChan <- nil // <--
		close(errChan) // <--
	}()

	// vvv
	if copyErr := <-errChan; copyErr != nil {
		fmt.Printf("WRITE ERROR: %v\n", copyErr)
		return
	}
	// ^^^

	for ok < 10 {
		var buf [1]byte
		n, err := io.ReadFull(buffer, buf[:])
		if err != nil || n != len(buf) {
			fmt.Printf("READ ERR. i: %d. n: %d. error: %v\n", ok, n, err)
			continue
		}
		if ok != int(buf[0]-'0') {
			fmt.Printf("ERROR: index: %d. b: %v\n", ok, buf[0])
		} else {
			fmt.Printf("OK: index: %d. b: %v\n", ok, buf[0])
		}
		ok++
	}
}

Output:

% go run -race foo.go
OK: index: 0. b: 48
OK: index: 1. b: 49
OK: index: 2. b: 50
OK: index: 3. b: 51
OK: index: 4. b: 52
OK: index: 5. b: 53
OK: index: 6. b: 54
OK: index: 7. b: 55
OK: index: 8. b: 56
OK: index: 9. b: 57

If you remove the <-errChan, then there is a race. But that's not what we're doing. Closing, but please feel free to comment if you think I've missed something.

[kolyshkin]

BTW since #2799 we do run all unit and int tests in GHA with race detection enabled, which should catch bugs like this.