Phone:

Hidden from the page source until you click: friction against scrapers, not a guarantee.

Email:

[email protected]

Category:

Go

Tags:
  • go
  • sync-pool
  • memory-safety
  • security
  • concurrency
  • debugging

Go's sync.Pool: Why Pooled Buffers Leak Data Between Requests

Here is a function that looks perfectly reasonable. It borrows a buffer from a pool, encodes some JSON into it, gives the buffer back, and returns the bytes:

var bufPool = sync.Pool{
	New: func() any { return new(bytes.Buffer) },
}

func encode(v any) ([]byte, error) {
	buf := bufPool.Get().(*bytes.Buffer)
	buf.Reset()
	defer bufPool.Put(buf)

	if err := json.NewEncoder(buf).Encode(v); err != nil {
		return nil, err
	}
	return buf.Bytes(), nil
}

It passes tests. It passes code review, because every individual line is the pattern from the documentation. It will also, under load, occasionally hand one user's JSON to another user. Nothing panics and nothing shows up as an error; the wrong bytes just go out on the wire.

What the pool actually promises

The sync.Pool documentation is short and worth reading properly. Get returns an arbitrary item from the pool, or calls New if there is none. Put adds an item. That is the entire contract. There is no promise about what state the item is in, and no promise that an item you put in will ever come back (the pool may drop items at any garbage collection, and in race-detector builds it deliberately drops some at random).

Crucially, the pool does not clear anything. It stores pointers. If you put a buffer holding a session token in, the next Get returns the same memory with the same token still sitting in it. Everything that follows is a consequence of that one fact.

Bug one: the alias that outlives the Put

Back to encode. buf.Bytes() does not copy. The docs say the slice is valid only until the next buffer modification. The deferred Put runs after the return value is evaluated, so the caller receives a slice pointing into memory that is already back in the pool, and free for anyone else to grab.

You can watch it happen without any concurrency at all:

a, _ := encode(map[string]string{"user": "alice"})
b, _ := encode(map[string]string{"user": "bob"})
fmt.Printf("%q\n%q\n", a, b)

On a single goroutine with no GC in between, the second call normally gets the very same buffer back (the pool keeps a per-P private slot for exactly this). a is 17 bytes long, b is 15, and both point at the same backing array. So a comes out as {"user":"bob"}, a newline, then the last two stale bytes of alice's payload. Alice's slice now contains Bob's data, which is the bug in miniature.

With real traffic the overwrite happens from another goroutine at some arbitrary moment, which is why it is intermittent and why it tends to survive testing. Run with -race and you may catch it as a data race, but note the race build randomly discards pooled items, so it will also make the bug appear less often. Flaky in both directions.

The fix is to decide who owns the memory. Either copy before returning:

return bytes.Clone(buf.Bytes()), nil

or, usually better, do not return bytes at all. Write straight to the destination while you hold the buffer, and only Put once nothing can still see it:

func writeJSON(w http.ResponseWriter, v any) error {
	buf := bufPool.Get().(*bytes.Buffer)
	buf.Reset()
	defer bufPool.Put(buf)

	if err := json.NewEncoder(buf).Encode(v); err != nil {
		return err
	}
	_, err := w.Write(buf.Bytes())
	return err
}

Here w.Write must not retain the slice, and the io.Writer contract says it must not. A ResponseWriter is fine; a hand-rolled writer that queues your slice for later is not, so be wary when the writer is something you did not write.

Bug two: the same thing via goroutines and channels

The alias does not need to be a return value. Anything that keeps a reference past the Put does it: sending buf.Bytes() down a channel to a logging goroutine, stashing a sub-slice in a struct, handing it to an async queue "for later". Sub-slices are the sneaky one. If you parse a request into a pooled buffer and then keep buf[10:20] as a "username" string field, you are holding a window into memory that will be rewritten. Converting with string(b) copies, so that is safe; unsafe.String tricks that avoid the copy are exactly the ones that bite here.

Bug three: forgetting the reset, or resetting the wrong way

The obvious one first. If you Get a bytes.Buffer and skip Reset, your output starts with whatever the last user left. Easy to spot. Put the Reset in one place, either on Get or on Put, and stick to it. I prefer doing it just before Put, so nothing sits in the pool holding data, but the important thing is that a single helper does it and the call sites never think about it.

The less obvious version is with raw byte slices, where people reslice to full capacity:

bp := slicePool.Get().(*[]byte)
b := (*bp)[:cap(*bp)] // whole buffer, previous contents included
n, err := conn.Read(b)
if err != nil {
	return err
}
process(b) // BUG: should be b[:n]

A short read fills the first n bytes and leaves the rest as it was. Passing b instead of b[:n] processes, and quite possibly echoes back, the tail of someone else's earlier message. Same family of mistake as sending a whole fixed-size array when only part of it was filled, and the classic way that a "heartbleed-shaped" bug appears in memory-safe languages: no out-of-bounds access, just in-bounds bytes that belonged to somebody else.

Also, actually, Reset itself is only a length change. bytes.Buffer.Reset sets the length to zero and keeps the backing array, with all the old bytes still in it. That is fine for correctness as long as you only ever read what you wrote, but it matters for the next section.

Bug four: secrets that never go away

Suppose the pooled buffer held a password, an API key or a decrypted body. After Reset and Put, those bytes sit in the heap until something overwrites them. If a later bug of the kinds above exposes the buffer, it exposes them too, and they will show up in any core dump or heap profile. For sensitive data, zero before returning:

func putSecret(bp *[]byte) {
	b := *bp
	clear(b[:cap(b)]) // built-in since Go 1.21
	*bp = b[:0]
	secretPool.Put(bp)
}

Two caveats. This zeroes only the buffer you are holding: if a bytes.Buffer grew while you wrote, the old, smaller backing array was abandoned with your data in it and you cannot reach it any more. For anything secret, allocate the right size up front, or do not pool it at all. Allocation is cheap in Go, and a fresh zeroed allocation is a perfectly good security feature. Also, the compiler is allowed to remove dead stores in some languages; Go's clear is a built-in with defined semantics, so that is not the worry here. Copies made elsewhere (by a library, by string(b)) are.

Should you be pooling at all?

Often, no. Pools earn their place when you have measured allocation pressure from large, short-lived, uniformly sized objects. The standard library does this in fmt, and it is instructive that fmt refuses to return buffers over 64 KiB to its pool: one enormous request would otherwise leave an enormous buffer pinned in the pool. If you pool without a size cap, one 50 MB response can sit in memory for the life of the process.

A reasonable checklist before reaching for sync.Pool:

  • Have you got a benchmark or heap profile showing the allocations matter?
  • Can a single function own the buffer from Get to Put, with nothing escaping?
  • Is there one reset helper, and a size cap on what goes back?
  • Does the data include anything you would mind a different user seeing?

If the third answer is "no" or the last one is "yes", skip the pool. The bug is not that sync.Pool is unsafe; it is that it hands you memory with a history, and it is entirely your job to make sure that history is never read.

← Back to the blog