GZIP: close underlying Writer

[flrdv]

GZIP writer doesn't close the underlying io.Writer when Close() is called. Unsure about flate and zstd.

This is a problem when writers are chained together.

I'm passing my chunked transfer-encoding writer to gzip.Writer#Reset() method, which relies on the Close() call to write the last zero-length chunk and flush the buffer.

There are a few workarounds, e.g. storing all chained writers in a slice nearby or wrapping each one, thereby building a tree/linked-list, enabling a way to manually close each one. However, I personally find them pretty ugly and would prefer the gzip.Writer#Close() to do the job.

I can open a PR if you agree on this.

[klauspost]

No. First of all, it is bad practice to close a provided stream. Even if you are done with the stream you have no idea what the caller intends. They may want to write more.

Second of all it would be breaking existing code.

Third, it would be incompatible with the standard library. And I don't think you would have much luck arguing this change to the Go team for the 2 reasons above.

You will have to deal with this, since you, as the caller, are responsible for closing the writers.

[flrdv]

Hm. I followed the logic that Close() is finito la comedia and that there's Flush(), which does exactly the same except not touching the provided stream.

So if closing the provided stream is considered a bad practice, what is considered a good one then? As mentioned before, storing a slice of chained writers?

[klauspost]

there's Flush(), which does exactly the same

No. There is a functional difference. Close() writes an EOF to stream, whereas Flush() writes out all received bytes, but expect more (and only rarely has to be used).

what is considered a good one

That is more of a "go standard library discussion". The basics are of course... (ignoring errors here)

func main() {
	out, _ := os.Create("out.txt")
	defer out.Close()
	w, _ := flate.NewWriter(out, flate.DefaultCompression)
	defer w.Close()
	w.Write([]byte("hello world"))
}

But you can easily do a wrapper for a io.WriteCloser that can do the closing in the order you want... Something like this...

// CloseWrapper can wrap one or more io.Closers to ensure they are closed when the Close method is called.
// If the reader or writer is nil, it will return an error when Read or Write is called.
// The Close method will close all closers in the order they were added.
// If any closer returns an error, it will be returned, but the others will still be closed.
// The CloseWrapper can be used as either an io.ReadCloser or io.WriteCloser, depending on the provided reader and writer.
type CloseWrapper struct {
	r io.Reader
	w io.Writer
	c []io.Closer
}

// NewCloseWrapper creates a new CloseWrapper that wraps the provided reader and writer.
func NewCloseWrapper(r io.Reader, w io.Writer, c ...io.Closer) *CloseWrapper {
	return &CloseWrapper{
		r: r,
		w: w,
		c: c,
	}
}

// Read implements the io.Reader interface.
func (cw *CloseWrapper) Read(p []byte) (n int, err error) {
	if cw.r == nil {
		return 0, errors.New("no reader")
	}
	return cw.r.Read(p)
}

// Write implements the io.Writer interface.
func (cw *CloseWrapper) Write(p []byte) (n int, err error) {
	if cw.w == nil {
		return 0, errors.New("no writer")
	}
	return cw.w.Write(p)
}

// Close implements the io.Closer interface.
func (cw *CloseWrapper) Close() error {
	var err error
	for _, c := range cw.c {
		if e := c.Close(); e != nil && err == nil {
			err = e
		}
	}
	return err
}

// AddFront will add a closer to the front of the list, that will be closed before any other currently added closers.
// If the closer is nil, it will not be added.
func (cw *CloseWrapper) AddFront(c io.Closer) {
	if c != nil {
		cw.c = append(cw.c, io.Closer(nil))
		copy(cw.c[1:], cw.c)
		cw.c[0] = c
	}
}

// AddBack will add a closer to the end of the list, that will be closed after any other currently added closers.
// If the closer is nil, it will not be added.
func (cw *CloseWrapper) AddBack(c io.Closer) {
	if c != nil {
		cw.c = append(cw.c, c)
	}
}

func (cw *CloseWrapper) SetWriter(w io.Writer) {
	cw.w = w
}

func (cw *CloseWrapper) SetReader(r io.Reader) {
	cw.r = r
}

Usage would look something like this:

func openFile() io.WriteCloser {
	var cw CloseWrapper
	out, _ := os.Create("out.txt")
	cw.AddFront(out)
	w, _ := flate.NewWriter(out, flate.DefaultCompression)
	cw.AddFront(w)
	cw.SetWriter(w)
	return &cw
}

(being a bit more careful to also close on errors)

You can trivially also change c []io.Closer to c []func() error and have even more flexibility.

[flrdv]

I honestly find this highly unfortunate, but there doesn't seem to be any other ways. I guess I’ll just stick to a similar way of tracking chained writers. Thank you.