compress/flate: Write() causes large and unshrinkable stack growth

[y3llowcake]

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.7.1 linux/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN="/home/cy/go/go-1.7.1/gopath/bin"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/cy/go/go-1.7.1/gopath"
GORACE=""
GOROOT="/home/cy/go/go-1.7.1"
GOTOOLDIR="/home/cy/go/go-1.7.1/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build525622235=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"

What did you do?

package main

import (
        "compress/flate"
        "fmt"
        "io/ioutil"
        "runtime"
        "runtime/debug"
)

const limit = 1024 * 16

func printss() {
        m := runtime.MemStats{}
        runtime.ReadMemStats(&m)
        fmt.Printf("inuse: %d sys: %d\n", m.StackInuse, m.StackSys)
}

func doflate() {
        z := make([]byte, limit*1000, limit*1000)
        fl, _ := flate.NewWriter(ioutil.Discard, 3)
        printss()
        fl.Write(z) // boom.
}

func main() {
        fmt.Println("hi")
        debug.SetMaxStack(limit) // comment out for more detail.
        doflate()                // boom.
        printss()
        runtime.GC()
        printss()
}

What did you expect to see?

Passing a large input to flate.Write() would not cause the stack to grow beyond the limit provided.

What did you see instead?

The stack grows very large.

Also, if I comment out this line:
debug.SetMaxStack(limit)

I get the following output:

hi
inuse: 393216 sys: 393216
inuse: 1474560 sys: 1474560
inuse: 950272 sys: 950272

Which suggests the runtime managed to shrink the stack a bit, but not nearly close enough to the original size.

Now imagine you have written a server that has lots of long running goroutines that occasionally compress a large blob. After introducing this code path, our stack usage increased ~10x.

[dsnet]

Wow. Fun bug. The problem is that we try to iterate over an array, which copies it to the stack. The arrays in question are hashHead and hashPrev, which we iterate over in these loops.

[gopherbot]

CL https://golang.org/cl/35122 mentions this issue.

[dsnet]

\cc @klauspost, I believe your library probably has the same bug.

[klauspost]

@dsnet - thanks for notifying me.

Alternatively we could allocated them. That would also make "Best Speed" faster, since it does not need these two buffers.

[cherrymui]

Having not looked at the source code, I wonder why the compiler makes such copy.

[dsnet]

Ironically, @dgryski had a twitter post about this, recently:

Today's #golang gotcha: the two-value range over an array does a copy. Avoid by ranging over the pointer instead.
https://play.golang.org/p/4b181zkB1O

[cherrymui]

@dsnet, thanks!
This is required by spec:

The range expression is evaluated once before beginning the loop...

So this comes from the difference between evaluating an array vs. evaluating a slice... Still, this behavior is somewhat surprising.

[valyala]

For those who wants small stacks with compress/* on go versions up to 1.7 - try using https://godoc.org/github.com/valyala/fasthttp/stackless . This is an ugly workaround we use in our production servers :) It just creates GOMAXPROCS goroutines dedicated for calling Write method on compress/* writers and passes Write calls to these goroutines via channels.

[minux]

On Jan 12, 2017 2:09 PM, "cherrymui" <[email protected]> wrote: @dsnet <https://github.com/dsnet>, thanks! This is required by spec: The range expression is evaluated once before beginning the loop... So this comes from the difference between evaluating an array vs. evaluating a slice... Still, this behavior is somewhat surprising. I think the compiler can optimize out the copy if it can prove that the body doesn't modify the array. But I'm not sure whether it's easy to determine that or not. It needs aliasing analysis.

[klauspost]

Using "BestSpeed" (level 1) should AFAICT solve the issue for Go 1.7.