From e82d152e66e81812a5c6ebf075cc99efd2602b19 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Tue, 18 Sep 2018 12:52:39 -0700 Subject: [PATCH] fmt: fix usage of sync.Pool The current usage of sync.Pool is leaky because it stores an arbitrary sized buffer into the pool. However, sync.Pool assumes that all items in the pool are interchangeable from a memory cost perspective. Due to the unbounded size of a buffer that may be added, it is possible for the pool to eventually pin arbitrarily large amounts of memory in a live-lock situation. As a simple fix, we just set a maximum size that we permit back into the pool. We do not need to fix the use of a sync.Pool in scan.go since the free method has always enforced a maximum capacity since the first commit of the scan logic. Fixes #27740 Updates #23199 Change-Id: I875278f7dba42625405df36df3e9b028252ce5e3 Reviewed-on: https://go-review.googlesource.com/136116 Reviewed-by: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Gobot Gobot --- src/fmt/print.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/fmt/print.go b/src/fmt/print.go index c9d694b07d..32743d0712 100644 --- a/src/fmt/print.go +++ b/src/fmt/print.go @@ -139,6 +139,16 @@ func newPrinter() *pp { // free saves used pp structs in ppFree; avoids an allocation per invocation. func (p *pp) free() { + // Proper usage of a sync.Pool requires each entry to have approximately + // the same memory cost. To obtain this property when the stored type + // contains a variably-sized buffer, we add a hard limit on the maximum buffer + // to place back in the pool. + // + // See https://golang.org/issue/23199 + if cap(p.buf) > 64<<10 { + return + } + p.buf = p.buf[:0] p.arg = nil p.value = reflect.Value{}