go icon indicating copy to clipboard operation
go copied to clipboard

proposal: io: add CloserFunc and WriterAndCloser

Open mei-rune opened this issue 2 years ago • 4 comments

can add CloserFunc and WriterAndCloser

CloserFunc is:

   type CloserFunc  func() error
   
   func (fn CloserFunc) Close() error {
      return fn()
   }

 var _ io.Closer = CloserFunc(func() error {})

WriterAndCloser is:

type writeCloser struct {
	io.Writer
	io.Closer	
}

func WriterAndCloser(w io.Writer, c io.Closer) io.WriteCloser {
	return writeCloser{
		Writer: w,
		Closer: c,
	}
}

see: https://github.com/search?q=CloserFunc+language%3Ago&type=code

mei-rune avatar Oct 21 '22 12:10 mei-rune

Your GitHub search seems to find a lot of cases of functions named CloserFunc, but that don't, and shouldn't, have a Close method. Can you point us to a few existing examples of code that would benefit from these additions? Thanks.

ianlancetaylor avatar Oct 21 '22 17:10 ianlancetaylor

@ianlancetaylor I found the close method in the 12 files in the first 3 pages of search results.

https://github.com/angry-tony/cortex-Prometheus-as-a-Service/blob/6d4678127598d65fe6405d4120f9911839fb00b6/pkg/ring/kv/etcd/mock.go#L70 https://github.com/go-kata/kdone/blob/edbd113fc705bc1d735d39f01b6429820bf5341a/closer.go https://github.com/tgulacsi/go/blob/60a9e8c66e45d6e5c6571574b55bbd8f6c43d73e/iohlp/multicloser.go#L52 https://github.com/cloudradar-monitoring/tacoscript/blob/5bd90febe845b8b1f688f7f8bef2eb154f1fd101/utils/closer.go#L28 https://github.com/goph/fxt/blob/eb299b7150bc0e7c19cbabe277643526e4c37e17/lifecycle.go#L28 https://github.com/HewlettPackard/k8s-sigstore-attestor/blob/5458d8b4a3e2622180959edc9e762bbd8badc56a/pkg/common/catalog/closers.go#L30 https://github.com/ranchercus/rancher/blob/42c0368ca1b8fc1510017c2bf7575634d35014d2/pkg/clusterprovisioninglogger/log.go#L81 https://github.com/chatrale/khan/blob/3ed340c0e005cbbd4e3f42e0ff93bce9cf3d4123/iplist/packed.go#L117 https://github.com/MrE-Fog/acr-cli11/blob/5df1cc9da4197e67e770585d7d863bac75279f86/vendor/oras.land/oras-go/v2/internal/ioutil/io.go#L31 https://github.com/goph/stdlib/blob/426371efcfd69ba2b2bde885e973aeede7b78e10/ext/close.go#L20 https://github.com/djbanodkar/RancherTesting/blob/88bec4200cfe5738495c4c8291ad6b1c521dc1d8/pkg/clusterprovisioninglogger/log.go#L81 https://github.com/manasmishra/spire/blob/355ab564b8d73ce8d1f17b5db7f8b9edbc2472a2/pkg/common/catalog/closers.go#L32

mei-rune avatar Oct 22 '22 05:10 mei-rune

I think this should wait until there's a negative resolution of #21670 / #25860 / #47487.

earthboundkid avatar Oct 24 '22 13:10 earthboundkid

This seems like two separate proposals which happen to compose together but which could be discussed independently.

I agree that the first part -- CloserFunc -- seems to be subsumable into all of the various proposals to allow more convenient use of isolated functions to implement interfaces, and so a decision on that should consider the outcome of a decision on those other proposals.

The WriterAndCloser part seems interesting in its own right though, and is similar to a situation I've encountered a small number of times: wrapping another function which returns an io.Reader and hooking an additional Close side-effect onto it, to encapsulate some additional cleanup that needs to happen when passing the result to a function which accepts any arbitrary io.ReadCloser.

When I most recently had that need I optimistically looked into the io package for something with a similar but not quite identical design as this proposal:

func ReaderWithClose(r io.Reader, close func () error) io.ReadCloser

...which is a similar idea as the existing io.NopCloser but with an additional hook function to call in Close, rather than just immediately return nil.

I ended up just implementing this myself, since of course the implementation was pretty trivial for my specific case:

type readerWithClose struct {
	io.Reader
	close func() error
}

func ReaderWithClose(r io.Reader, close func() error) io.ReadCloser {
	return readerWithClose{
		Reader: r,
		close:  close,
	}
}

func (r readerWithClose) Close() error {
	return r.close()
}

I considered generalizing it into a reusable library but found some interesting questions when considering use-cases beyond my immediate need:

  • What should happen if the given io.Reader is also already an io.Closer? Does the new Close method call both the close callback and the underlying reader's Close? If so, what happens if one of them fails and the other succeeds, or if both of them fail?

  • What about all of the other optional interfaces the given reader could possibly implement? I see that io.NopCloser itself already contains a special case for when the given reader is an io.WriterTo:

    func NopCloser(r Reader) ReadCloser {
    	if _, ok := r.(WriterTo); ok {
    		return nopCloserWriterTo{r}
    	}
    	return nopCloser{r}
    }
    

    I assume that this hypothetical ReaderWithClose would need to do something similar in order to preserve the io.Copy optimization that io.WriterTo implies.

Of course I realize that I'm talking about readers while the original proposal was talking about writers, and so my details here are off-topic. But I'm sharing this because I think there are equivalent questions for io.WriterAndCloser (as proposed here) or io.WriterWithClose (a write variant of my ReaderWithClose), and because it seems to me that if this proposal were accepted for writers then it would make sense to accept it for readers too, for consistency/symmetry.

(The question of whether the second argument to this function should be just a function or an io.Closer of course amounts to re-asking the question about the io.CloserFunc part of this proposal; I don't feel strongly either way about this and am intending to focus only on the questions that seem to arise when hooking an extra Close behavior onto an existing object, regardless of the exact interface used to do so.)

apparentlymart avatar Oct 27 '22 18:10 apparentlymart