bufio + net: memory leak

[gpaul]

This follows from issue #5314

go version:
go version devel +58f8a30f5b78 Tue Apr 16 14:20:06 2013 -0700 linux/amd64

uname -a:
Linux me 3.3.8-1.fc16.x86_64 #1 SMP Mon Jun 4 20:49:02 UTC 2012 x86_64 x86_64 x86_64
GNU/Linux

Code to reproduce is here:
http://play.golang.org/p/7tTgtZHa-F

It creates a bunch of goroutines that block in (*bufio.Reader).Read from a blocking
net.Conn. These never die. They call Handle() which allocates and cuts apart a 64MB byte
slice, then returns. It takes no input parameters and returns nothing, so all garbage in
it should be collected.

This does not happen though, the heap profile grows and says nearly everything was
allocated in the Handle() and never freed.

Running with GOGCTRACE=1 shows the heap grow.

[davecheney]

Comment 1:

Issue #5314 has been merged into this issue.

[gpaul]

Comment 2:

So weird. If I do a plain read from the reader, no problem, if I do an ioutil.ReadAll,
it brakes.
reader := bufio.NewReader(conn)
followed by
_, err = reader.Read(make([]byte, 1))
vs.
_, err = ioutil.ReadAll(reader)

[gpaul]

Comment 3:

attached output and heap profile
code is:
http://play.golang.org/p/bJF0etsZaE

Attachments:

1. mem.svg (10218 bytes)
2. gc.txt (12161 bytes)

[gpaul]

Comment 4:

Two pmaps about a minute apart:

Attachments:

1. first.pmap (1910 bytes)
2. second.pmap (1953 bytes)

[gpaul]

Comment 5:

This combination leaks:
reader := bufio.NewReader(conn)
_, err = ioutil.ReadAll(reader)
However, using only ioutil.ReadAll straight off the conn leads to no leak.
            
_, err = ioutil.ReadAll(conn)
Similarly, using only the bufio Read leads to no leak either.
reader := bufio.NewReader(conn)
reader.Read(make([]byte, 1))
As you can see from the gc.txt attachment, it doesn't leak on every loop, only every Nth
loop.

[davecheney]

Comment 6:

> This combination leaks:
> 
> reader := bufio.NewReader(conn)
> _, err = ioutil.ReadAll(reader)
Does ioutil.ReadAll try to sense for the presence of ReadFrom/WriteTo ?
What happens if you wrap reader in your own type that masks off the ReadFrom/WriteTo
methods of bufio.Reader ?

Status changed to WaitingForReply.

[gpaul]

Comment 7:

Yay! Reduced the example to use os.Pipe to create a blocking fd read instead of the net
package. Its a pretty simple function now, how someone can have a look.

Attachments:

1. ram_test.go (648 bytes)

[bradfitz]

Comment 8:

I don't see any bug here.
The writer is never closed, so the reads will block forever, and the goroutines will
leak and waste memory. Working as intended.
Yes, the memory from Handle might not be collected as aggressively as possible, but
that's the stack-precise GC that Carl is working on in issue #5134.
But this program in comment #7 still has a leak.

Status changed to Invalid.

[gpaul]

Comment 9:

Thanks Brad,
I didn't know we didn't do stack-precise GC - all this makes way more sense now.
I look forward to Carl's patches.