cache: use finalizer for freeing manually allocated memory

Currently, the `entryCacheAlloc` struct maintains a slice of `entry`
structs. When CGo is enabled, the latter is backed by manually allocated
memory. As the former is a pooled struct, when it is released from the
pool, it will be garbage collected by the Go runtime.

The patch in #1087 introduced a regression where a no-op finalizer is
used unless running with `invariants` or `trace` when `race` is
disabled.  Typical production binaries will not have these flags set, so
the no-op finalizer is installed in place of a "real" finalizer. This
results in a small memory leak given that the memory backing each of the
`entry` items is not released when the pool releases an
`entryCacheAlloc` and it is garbage collected by the runtime.

Revert to installing the "real" finalizer in this case, as it is
required for correctness.

Update the documentation on the use of finalizers to point out that we
do indeed rely on them for correctness, in a very niche, but valid
usecase for freeing manually allocated memory.

Fixes #1588.

docs/memory.md

@@ -76,11 +76,17 @@ with very loose guarantees:
 > typically they are useful only for releasing non-memory resources
 > associated with an object during a long-running program
 
-This language is somewhat frightening, but in practice finalizers are
-run at the end of every GC period. Pebble does not use finalizers for
-correctness, but instead uses them for its leak detection facility. In
-the block cache, a finalizer is associated with the Go allocated
-`cache.Value` object. When the finalizer is run, it checks that the
-buffer backing the `cache.Value` has been freed. This leak detection
-facility is enabled by the `"invariants"` build tag which is enabled
-by the Pebble unit tests.
+This language is somewhat frightening, but in practice finalizers are run at the
+end of every GC period. Pebble primarily relies on finalizers for its leak
+detection facility. In the block cache, a finalizer is associated with the Go
+allocated `cache.Value` object. When the finalizer is run, it checks that the
+buffer backing the `cache.Value` has been freed. This leak detection facility is
+enabled by the `"invariants"` build tag which is enabled by the Pebble unit
+tests.
+
+There also exists a very specific memory reclamation use case in the block cache
+that ensures that structs with transitively reachable fields backed by manually
+allocated memory that are pooled in a `sync.Pool` are freed correctly when their
+parent struct is released from the pool and consequently garbage collected by
+the Go runtime (see `cache/entry_normal.go`). The loose guarantees provided by
+the runtime are reasonable to rely on in this case to prevent a memory leak.

internal/cache/entry_normal.go

@@ -7,6 +7,7 @@
 package cache
 
 import (
+	"runtime"
 	"sync"
 	"unsafe"
 
@@ -53,9 +54,12 @@ type entryAllocCache struct {
 func newEntryAllocCache() *entryAllocCache {
 	c := &entryAllocCache{}
 	if !entriesGoAllocated {
-		// Note: this is a no-op if invariants and tracing are disabled or race is
-		// enabled.
-		invariants.SetFinalizer(c, freeEntryAllocCache)
+		// Note the use of a "real" finalizer here (as opposed to a build tag-gated
+		// no-op finalizer). Without the finalizer, objects released from the pool
+		// and subsequently GC'd by the Go runtime would fail to have their manually
+		// allocated memory freed, which results in a memory leak.
+		// lint:ignore SetFinalizer
+		runtime.SetFinalizer(c, freeEntryAllocCache)
 	}
 	return c
 }

internal/lint/lint_test.go

@@ -120,7 +120,8 @@ func TestLint(t *testing.T) {
 
 		if err := stream.ForEach(
 			stream.Sequence(
-				dirCmd(t, pkg.Dir, "git", "grep", "runtime\\.SetFinalizer("),
+				dirCmd(t, pkg.Dir, "git", "grep", "-B1", "runtime\\.SetFinalizer("),
+				lintIgnore("lint:ignore SetFinalizer"),
 				stream.GrepNot(`^vendor/`), // ignore vendor
 				stream.GrepNot(`^internal/invariants/finalizer_on.go`),
 			), func(s string) {
@@ -194,3 +195,32 @@ func TestLint(t *testing.T) {
 		}
 	})
 }
+
+// lintIgnore is a stream.FilterFunc that filters out lines that are preceded by
+// the given ignore directive. The function assumes the input stream receives a
+// sequence of strings that are to be considered as pairs. If the first string
+// in the sequence matches the ignore directive, the following string is
+// dropped, else it is emitted.
+//
+// For example, given the sequence "foo", "bar", "baz", "bam", and an ignore
+// directive "foo", the sequence "baz", "bam" would be emitted. If the directive
+// was "baz", the sequence "foo", "bar" would be emitted.
+func lintIgnore(ignore string) stream.FilterFunc {
+	return func(arg stream.Arg) error {
+		var prev string
+		var i int
+		for s := range arg.In {
+			if i%2 == 0 {
+				// Fist string in the pair is used as the filter. Store it.
+				prev = s
+			} else {
+				// Second string is emitted only if it _does not_ match the directive.
+				if !strings.Contains(prev, ignore) {
+					arg.Out <- s
+				}
+			}
+			i++
+		}
+		return nil
+	}
+}