sstable: detect double Close on sstable iterators

A flag indicates if an iterator is in the pool; if we call `Close()`
twice and the iterator has not been reused yet, we panic.

We add a `CloseHelper` which can absorb multiple `Close()` calls,
which is convenient to use with `defer`.

Also update the `InternalIterator` documentation to reflect the
reality that multiple `Close()` calls are not supported by all
implementations.

internal/base/close_helper.go

@@ -0,0 +1,30 @@
+// Copyright 2024 The LevelDB-Go and Pebble Authors. All rights reserved. Use
+// of this source code is governed by a BSD-style license that can be found in
+// the LICENSE file.
+
+package base
+
+import "io"
+
+// CloseHelper wraps an io.Closer in a wrapper that ignores extra calls to
+// Close. It is useful to ensure cleanup in error paths (using defer) without
+// double-closing.
+func CloseHelper(closer io.Closer) io.Closer {
+	return &closeHelper{
+		Closer: closer,
+	}
+}
+
+type closeHelper struct {
+	Closer io.Closer
+}
+
+// Close the underlying Closer, unless it was already closed.
+func (h *closeHelper) Close() error {
+	closer := h.Closer
+	if closer == nil {
+		return nil
+	}
+	h.Closer = nil
+	return closer.Close()
+}

internal/base/iterator.go

@@ -191,8 +191,10 @@ type InternalIterator interface {
 
 	// Close closes the iterator and returns any accumulated error. Exhausting
 	// all the key/value pairs in a table is not considered to be an error.
-	// It is valid to call Close multiple times. Other methods should not be
-	// called after the iterator has been closed.
+	//
+	// Once Close is called, the iterator should not be used again. Specific
+	// implementations may support multiple calls to Close (but no other calls
+	// after the first Close).
 	Close() error
 
 	// SetBounds sets the lower and upper bounds for the iterator. Note that the

sstable/reader_iter_single_lvl.go

@@ -165,6 +165,10 @@ type singleLevelIterator struct {
 	lastBloomFilterMatched bool
 
 	hideObsoletePoints bool
+
+	// inPool is set to true before putting the iterator in the reusable pool;
+	// used to detect double-close.
+	inPool bool
 }
 
 // singleLevelIterator implements the base.InternalIterator interface.
@@ -203,6 +207,7 @@ func (i *singleLevelIterator) init(
 		i.endKeyInclusive, lower, upper = v.constrainBounds(lower, upper, false /* endInclusive */)
 	}
 
+	i.inPool = false
 	i.ctx = ctx
 	i.lower = lower
 	i.upper = upper
@@ -274,8 +279,9 @@ func (i *singleLevelIterator) setupForCompaction() {
 
 func (i *singleLevelIterator) resetForReuse() singleLevelIterator {
 	return singleLevelIterator{
-		index: i.index.resetForReuse(),
-		data:  i.data.resetForReuse(),
+		index:  i.index.resetForReuse(),
+		data:   i.data.resetForReuse(),
+		inPool: true,
 	}
 }
 
@@ -1440,6 +1446,9 @@ func firstError(err0, err1 error) error {
 // Close implements internalIterator.Close, as documented in the pebble
 // package.
 func (i *singleLevelIterator) Close() error {
+	if invariants.Enabled && i.inPool {
+		panic("Close called on interator in pool")
+	}
 	i.iterStats.close()
 	var err error
 	if i.closeHook != nil {

sstable/reader_iter_two_lvl.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 
 	"github.com/cockroachdb/pebble/internal/base"
+	"github.com/cockroachdb/pebble/internal/invariants"
 	"github.com/cockroachdb/pebble/objstorage/objstorageprovider/objiotracing"
 )
 
@@ -165,6 +166,7 @@ func (i *twoLevelIterator) init(
 		i.endKeyInclusive, lower, upper = v.constrainBounds(lower, upper, false /* endInclusive */)
 	}
 
+	i.inPool = false
 	i.ctx = ctx
 	i.lower = lower
 	i.upper = upper
@@ -979,6 +981,9 @@ func (i *twoLevelIterator) skipBackward() (*InternalKey, base.LazyValue) {
 // Close implements internalIterator.Close, as documented in the pebble
 // package.
 func (i *twoLevelIterator) Close() error {
+	if invariants.Enabled && i.inPool {
+		panic("Close called on interator in pool")
+	}
 	i.iterStats.close()
 	var err error
 	if i.closeHook != nil {

tool/sstable.go

@@ -391,7 +391,8 @@ func (s *sstableT) runScan(cmd *cobra.Command, args []string) {
 			fmt.Fprintf(stderr, "%s%s\n", prefix, err)
 			return
 		}
-		defer iter.Close()
+		iterCloser := base.CloseHelper(iter)
+		defer iterCloser.Close()
 		key, value := iter.SeekGE(s.start, base.SeekGEFlagsNone)
 
 		// We configured sstable.Reader to return raw tombstones which requires a
@@ -532,7 +533,7 @@ func (s *sstableT) runScan(cmd *cobra.Command, args []string) {
 			}
 		}
 
-		if err := iter.Close(); err != nil {
+		if err := iterCloser.Close(); err != nil {
 			fmt.Fprintf(stdout, "%s\n", err)
 		}
 	})