diff --git a/internal/store/pebble.go b/internal/store/pebble.go index e6395d2..5dd0c1c 100644 --- a/internal/store/pebble.go +++ b/internal/store/pebble.go @@ -67,6 +67,13 @@ type PebbleStore struct { // where the previous one stopped, wrapping at the end. frontierCursorMu sync.Mutex frontierCursor []byte + + // PILOT-190: pebble.DB.Close() panics if called twice. Wrap teardown + // in sync.Once so repeated Close() calls (e.g. from layered cleanups + // or signal handlers) are idempotent. closeErr caches the first + // Close result so every caller sees the same outcome. + closeOnce sync.Once + closeErr error } const ( @@ -188,7 +195,16 @@ func OpenPebble(path string) (*PebbleStore, error) { } // Close flushes and closes the underlying Pebble DB. -func (p *PebbleStore) Close() error { return p.db.Close() } +// +// Idempotent: pebble.DB.Close() panics if called twice (PILOT-190), so +// the teardown is wrapped in sync.Once. Subsequent calls are no-ops and +// return the same error as the first call. +func (p *PebbleStore) Close() error { + p.closeOnce.Do(func() { + p.closeErr = p.db.Close() + }) + return p.closeErr +} // Checkpoint creates a consistent, point-in-time snapshot of the DB at destDir // via hard-links to live SSTs. Cheap (no data copy) and safe to tar afterwards diff --git a/internal/store/pebble_test.go b/internal/store/pebble_test.go index 3694269..cf74ad5 100644 --- a/internal/store/pebble_test.go +++ b/internal/store/pebble_test.go @@ -661,3 +661,32 @@ func TestPebbleRecrawlURL(t *testing.T) { t.Errorf("missing URL: want ErrNotFound, got %v", err) } } + +// TestPebbleStore_CloseIsIdempotent — regression for PILOT-190. +// pebble.DB.Close() panics if called twice. PebbleStore wraps it in +// sync.Once so repeated Close() calls are safe and return the same +// error. Opens a store, calls Close() three times, asserts no panic +// and that every call returns the same value as the first. +func TestPebbleStore_CloseIsIdempotent(t *testing.T) { + // Skip newPebbleStore — its t.Cleanup() would also call Close() + // and we want to control invocation count explicitly. + dir := filepath.Join(t.TempDir(), "pebble") + p, err := OpenPebble(dir) + if err != nil { + t.Fatalf("OpenPebble: %v", err) + } + + defer func() { + if r := recover(); r != nil { + t.Fatalf("Close() panicked on repeated invocation: %v", r) + } + }() + + first := p.Close() + for i := 2; i <= 3; i++ { + got := p.Close() + if got != first { + t.Fatalf("Close() call #%d returned %v; want %v (cached first result)", i, got, first) + } + } +}