bufio: altered value when reading from tcpstream

[bench]

• goversion "1.5"
• arch "debian8-linux-amd64"
• What did I do? "Read line from TCP stream"

I received altered data when reading from tcpstream.
Pointer receivedAAA is partially overwritten by pointer receivedBBB. (side effect of readSlice()?)
Only appears on unit testing.
Works fine with go1.4.2, go 1.4.3.

Here is a way to reproduce the issue:

package hello

import (
    "bufio"
    "fmt"
    "net"
    "os"
    "testing"
)

const (
    str1 = "AAAAAAAAAAAA"
    str2 = "BBBB"
)

func TestGetCommand(t *testing.T) {

    // Run TCP Server
    address, err := net.ResolveTCPAddr("tcp", fmt.Sprintf("0.0.0.0:%d", 30299))
    if err != nil {
        panic(fmt.Sprintf("Error starting message server: %s", err))
    }
    l, err := net.ListenTCP("tcp", address)
    if err != nil {
        panic(fmt.Sprintf("Error starting message server: %s", err))
    }
    defer l.Close()

    go func() {
        for {
            // Listen for an incoming connection.
            conn, err := l.AcceptTCP()
            if err != nil {
                // Connection closed
                os.Exit(0)
            }
            _, err = fmt.Fprintf(conn, str1+"\n")
            if err != nil {
                panic(fmt.Sprintf("Error sending session ID: %v", err))
            }
            _, err = fmt.Fprintf(conn, str2+"\n")
            if err != nil {
                panic(fmt.Sprintf("Error sending session ID: %v", err))
            }
            conn.Close()
        }
    }()

    // TCP client
    addr, err := net.ResolveTCPAddr("tcp", "localhost:30299")
    if err != nil {
        t.Error("Error resolving address: %v", err)
    }
    connection, err := net.DialTCP("tcp", nil, addr)
    if err != nil {
        t.Error("Error during dial : %v", err)
    }
    defer connection.Close()

    reader := bufio.NewReader(connection)
    receivedStr1, _, err := reader.ReadLine()

    if err != nil {
        t.Error(err)
    }

    receivedStr2, _, err := reader.ReadLine()
    if err != nil {
        t.Error(err)
    }

    fmt.Printf("VALUE -- type %T: q=%p\n", receivedStr1, receivedStr1)
    fmt.Printf("VALUE -- type %T: q=%p\n", receivedStr2, receivedStr2)

    if string(receivedStr1) != str1 {
        t.Error("Bad Str1, should receive " + str1 + " but got " + string(receivedStr1))
    }
    if string(receivedStr2) != str2 {
        t.Error("Bad Str2, should receive " + str2 + " but got " + string(receivedStr2))
    }

}

Output

?       github.com/myapp    [no test files]
=== RUN   TestGetCommand
VALUE -- type []uint8: q=0xc8200a8000
VALUE -- type []uint8: q=0xc8200a8000
--- FAIL: TestGetCommand (0.00s)
    hello_test.go:76: Bad Str1, should receive AAAAAAAAAAAA but got BBBB
        AAAAAAA
FAIL
ok      github.com/myapp/hello  0.002s

Error seems to be due to client bufio, since tcp dump is correct

####
T 127.0.0.1:30299 -> 127.0.0.1:43386 [AP]
  AAAAAAAAAAAA.                                                                                                   
##
T 127.0.0.1:30299 -> 127.0.0.1:43386 [AP]
  BBBB.                                                                                                           
####

[davecheney]

I'm sorry, we cannot accept an issue report with zero error checking.
Please add appropriate error handling to your program.

On Fri, 22 Jan 2016, 00:04 Benjamin Chenebault notifications@github.com
wrote:

• goversion "1.5"
• arch "debian8-linux-amd64"
• What did I do? "Read line from TCP stream"

I received altered data when reading from tcpstream.
Pointer receivedAAA is partially overwritten by pointer receivedBBB. (side
effect of readSlice()?)
Only appears on unit testing.
Works fine with go1.4.2, go 1.4.3.

Here is a way to reproduce the issue:

package hello

import (
"bufio"
"fmt"
"net"
"testing"
)

func TestBug(t *testing.T) {

AAA := "AAAAAAAAAAAA\n"
BBB := "BBBB\n"

// TCP Server
address, _ := net.ResolveTCPAddr("tcp", fmt.Sprintf("0.0.0.0:%d", 30299))
l, _ := net.ListenTCP("tcp", address)

go func() {
    for {
        // Listen for an incoming connection.
        conn, _ := l.AcceptTCP()
        fmt.Fprintf(conn, AAA)
        fmt.Fprintf(conn, BBB)
        conn.Close()
    }
}()

// TCP client
addr, _ := net.ResolveTCPAddr("tcp", "localhost:30299")
connection, _ := net.DialTCP("tcp", nil, addr)
defer connection.Close()

reader := bufio.NewReader(connection)
receivedAAA, _ := reader.ReadSlice('\n')
receivedBBB, _ := reader.ReadSlice('\n')

fmt.Printf("VALUE -- type %T: q=%p\n", receivedAAA, receivedAAA)
fmt.Printf("VALUE -- type %T: q=%p\n", receivedBBB, receivedBBB)

if string(receivedAAA) != AAA {
    t.Error("should receive " + AAA + " but got " + string(receivedAAA))
}
if string(receivedBBB) != BBB {
    t.Error("should receive " + BBB + " but got " + string(receivedBBB))
}

}

Error seems to be due to client bufio, since tcp dump is correct

T 127.0.0.1:30299 -> 127.0.0.1:43386 [AP]
AAAAAAAAAAAA.

T 127.0.0.1:30299 -> 127.0.0.1:43386 [AP]
BBBB.

—
Reply to this email directly or view it on GitHub
#14055.

[bench]

Sorry, issue updated.

[davecheney]

@BenC- thanks for your issue report, i've confirmed this is a possible issue related to inlining or escape analysus. Running the sample program with -gcflags=-l resolves the issue. This is at revision 505fa7b which was around the beta2 release,

However, at 33a784e the issue is gone. I suspect that this was fixed by @dr2chase 's recent escape analysis fixes.

[davecheney]

@BenC- if you are able, you can try building master from source to verify the fix. If you don't want to build form source, the next beta or rc candidate should be out in a week or so.

[bench]

@davecheney Maybe i'm wrong but the issue seems to persist an in at 33a784e.
The error is non deterministic. Sometimes it works, sometimes it fails...

Failure cases below.

[GoProject:myapp]bchenebault@bchenebault ~/myapp> go version
go version devel +33a784e Thu Jan 21 01:35:07 2016 +0000 linux/amd64
[GoProject:myapp]bchenebault@bchenebault ~/myapp> go test -v github.com/...           
?       github.com/myapp    [no test files]
=== RUN   TestGetCommand
VALUE -- type []uint8: q=0xc820095000
VALUE -- type []uint8: q=0xc820095000
--- FAIL: TestGetCommand (0.00s)
    hello_test.go:76: Bad Str1, should receive AAAAAAAAAAAA but got BBBB
        AAAAAAA
FAIL
FAIL    github.com/myapp/hello  0.002s
[GoProject:myapp]bchenebault@bchenebault ~/myapp> go test -gcflags='-l' -v github.com/...                          1
?       github.com/myapp    [no test files]
=== RUN   TestGetCommand
VALUE -- type []uint8: q=0xc8200a1000
VALUE -- type []uint8: q=0xc8200a1000
--- FAIL: TestGetCommand (0.00s)
    hello_test.go:76: Bad Str1, should receive AAAAAAAAAAAA but got BBBB
        AAAAAAA
FAIL
FAIL    github.com/myapp/hello  0.002s

[bradfitz]

Program is invalid. It assumes that ReadLine returns a string (the "str" variables), but it's not a string. It's a slice of memory and you don't own it. See:

https://golang.org/pkg/bufio/#Reader.ReadLine

The returned buffer is only valid until the next call to ReadLine.

[bench]

Well, but this behavior implies a loss of backward compatibility since go1.5. But you'll reply I should not have trusted a pointer I don't own... ;) Sorry for the inconvenience.

[davecheney]

That we my mistake, sorry, I thought ReadLine returned a string not a
[]byte. But I guess that is why ReadLine was replaced with bufio.Scanner.

On Fri, 22 Jan 2016, 04:25 Benjamin Chenebault notifications@github.com
wrote:

OK got it. Sorry for the inconvenience.

—
Reply to this email directly or view it on GitHub
#14055 (comment).