golang/go

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

y3llowcake opened this issue · 10 comments

y3llowcake commented

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 commented

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.

  • 😄1
gopherbot commented

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

dsnet commented

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

  • 👍1
klauspost commented

@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 commented

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

dsnet commented

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 commented

@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 commented

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 commented
  • 👍1
klauspost commented

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