Use of []byte in sync.Pool

[johanbrandhorst]

Hi!

staticcheck flags the use of sync.Pool in server.go as incorrect, as it's passing []byte instead of *[]byte, which means every instance ends up allocating a slice header. See https://staticcheck.dev/docs/checks/#SA6002 and https://go-review.googlesource.com/c/go/+/24371 for more information, discussion and examples.

To fix it, I suggest changing instances of Put to pass a reference to the slice, and to ensure that the same alloc that was got is put back.

[miekg]

ah TIL. Care to make the PR too?

[johanbrandhorst]

I looked into this and I'm concerned that the existing use of pooling is buggy. If a user uses a custom Reader via srv.DecorateReader, we could end up endlessly putting slices into the pool without ever taking them out.

With regards to fixing the issue, I'm not sure it's going to be possible to do without breaking changes, since we'd need Reader.ReadUDP to return *[]byte instead of []byte to avoid allocating a slice header on every call to that function. I think we could close this issue and perhaps turn off pooling if a user uses DecorateReader to avoid the memory leak.