klauspost / pgzip

Go parallel gzip (de)compression
MIT License
1.12k stars 77 forks source link

gunzip: Reset may loose buffers #34

Closed buengese closed 4 years ago

buengese commented 4 years ago

If Reset is called on the gzip reader without reading the gzip stream untill EOF the read-ahead go-routine may loose buffers from the block pool.

Simple way to reproduce this is doing something like.

r, _ := pgzip.NewReader(in)
r.Reset(in)
io.Copy(ioutil.Discard, r)

Running something will succeed most of the time but sometimes it'll deadlock. What happens is that by the time killReadAhead() called the read ahead go-routine might have already taken a buffer from the block pool. This buffer is either send into the readAhead channel and is lost when this channel is reinitialized when doReadAhead is called again or it's lost when the go-routine exits in gunzip.go#L419. In both cases the buffer taken by the read-ahead go-routine is never returned to the block pool. If all buffers are lost the reader will completely deadlock.

The easiest way to fix this would probably be to reinitialize and fill the block pool in either Reset or doReadAhead

klauspost commented 4 years ago

@buengese I added a fix. If it is possible for you to confirm, I will make a prelease.

buengese commented 4 years ago

Can confirm this fixes my tests too.

klauspost commented 4 years ago

@buengese New release out.