From ef773cb2ff2aec681240bd76d8390d66f2e057c0 Mon Sep 17 00:00:00 2001 From: blindchaser Date: Wed, 4 Feb 2026 14:17:58 -0500 Subject: [PATCH 1/3] fix: prevent nil pointer panic during pruning on shutdown --- app/app.go | 9 ++------- sei-db/ss/pebbledb/db.go | 5 +++++ sei-db/ss/pebbledb/db_test.go | 26 ++++++++++++++++++++++++++ sei-db/ss/rocksdb/db.go | 5 +++++ sei-db/ss/rocksdb/db_test.go | 26 ++++++++++++++++++++++++++ 5 files changed, 64 insertions(+), 7 deletions(-) diff --git a/app/app.go b/app/app.go index d55affd6cf..bdc20be233 100644 --- a/app/app.go +++ b/app/app.go @@ -964,13 +964,8 @@ func (app *App) HandleClose() error { } } - // Close state store (SeiDB) - critical for cleaning up background goroutines - if app.stateStore != nil { - if err := app.stateStore.Close(); err != nil { - app.Logger().Error("failed to close state store", "error", err) - errs = append(errs, fmt.Errorf("failed to close state store: %w", err)) - } - } + // Note: stateStore (ssStore) is already closed by cms.Close() in BaseApp.Close() + // No need to close it again here. if len(errs) > 0 { return fmt.Errorf("errors during close: %v", errs) diff --git a/sei-db/ss/pebbledb/db.go b/sei-db/ss/pebbledb/db.go index 282f484bd2..3699518e6b 100644 --- a/sei-db/ss/pebbledb/db.go +++ b/sei-db/ss/pebbledb/db.go @@ -634,6 +634,11 @@ func (db *Database) writeAsyncInBackground() { // it has been updated. This occurs when that module's keys are updated in between pruning runs, the node after is restarted. // This is not a large issue given the next time that module is updated, it will be properly pruned thereafter. func (db *Database) Prune(version int64) (_err error) { + // Defensive check: ensure database is not closed + if db.storage == nil { + return errors.New("pebbledb: database is closed") + } + startTime := time.Now() defer func() { otelMetrics.pruneLatency.Record( diff --git a/sei-db/ss/pebbledb/db_test.go b/sei-db/ss/pebbledb/db_test.go index 075b80d35d..1dc58c4321 100644 --- a/sei-db/ss/pebbledb/db_test.go +++ b/sei-db/ss/pebbledb/db_test.go @@ -6,6 +6,7 @@ import ( "github.com/sei-protocol/sei-db/config" sstest "github.com/sei-protocol/sei-db/ss/test" "github.com/sei-protocol/sei-db/ss/types" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" ) @@ -22,3 +23,28 @@ func TestStorageTestSuite(t *testing.T) { suite.Run(t, s) } + +// TestPruneAfterClose verifies that calling Prune() after Close() returns an error +// instead of causing a panic due to nil pointer dereference. +// This is a regression test for the nil pointer panic during node shutdown. +func TestPruneAfterClose(t *testing.T) { + dir := t.TempDir() + cfg := config.DefaultStateStoreConfig() + cfg.Backend = "pebbledb" + + db, err := New(dir, cfg) + require.NoError(t, err) + + // Write some data + err = db.SetLatestVersion(1) + require.NoError(t, err) + + // Close the database + err = db.Close() + require.NoError(t, err) + + // Prune should return error, not panic + err = db.Prune(1) + require.Error(t, err) + require.Contains(t, err.Error(), "database is closed") +} diff --git a/sei-db/ss/rocksdb/db.go b/sei-db/ss/rocksdb/db.go index 0d0648e47d..e489f4e049 100644 --- a/sei-db/ss/rocksdb/db.go +++ b/sei-db/ss/rocksdb/db.go @@ -293,6 +293,11 @@ func (db *Database) writeAsyncInBackground() { // lazy prune. Future compactions will honor the increased full_history_ts_low // and trim history when possible. func (db *Database) Prune(version int64) error { + // Defensive check: ensure database is not closed + if db.storage == nil { + return fmt.Errorf("rocksdb: database is closed") + } + tsLow := version + 1 // we increment by 1 to include the provided version var ts [TimestampSize]byte diff --git a/sei-db/ss/rocksdb/db_test.go b/sei-db/ss/rocksdb/db_test.go index 605f377e62..aa86dd9a75 100644 --- a/sei-db/ss/rocksdb/db_test.go +++ b/sei-db/ss/rocksdb/db_test.go @@ -9,6 +9,7 @@ import ( "github.com/sei-protocol/sei-db/config" sstest "github.com/sei-protocol/sei-db/ss/test" "github.com/sei-protocol/sei-db/ss/types" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" ) @@ -25,3 +26,28 @@ func TestStorageTestSuite(t *testing.T) { suite.Run(t, s) } + +// TestPruneAfterClose verifies that calling Prune() after Close() returns an error +// instead of causing a panic due to nil pointer dereference. +// This is a regression test for the nil pointer panic during node shutdown. +func TestPruneAfterClose(t *testing.T) { + dir := t.TempDir() + cfg := config.DefaultStateStoreConfig() + cfg.Backend = "rocksdb" + + db, err := New(dir, cfg) + require.NoError(t, err) + + // Write some data + err = db.SetLatestVersion(1) + require.NoError(t, err) + + // Close the database + err = db.Close() + require.NoError(t, err) + + // Prune should return error, not panic + err = db.Prune(1) + require.Error(t, err) + require.Contains(t, err.Error(), "database is closed") +} From d9c1599df3612a1c287172f91f532887dc7f4b45 Mon Sep 17 00:00:00 2001 From: blindchaser Date: Wed, 4 Feb 2026 16:13:57 -0500 Subject: [PATCH 2/3] add atomic check --- sei-db/ss/pebbledb/db.go | 9 +++++++-- sei-db/ss/rocksdb/db.go | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/sei-db/ss/pebbledb/db.go b/sei-db/ss/pebbledb/db.go index 3699518e6b..a845d95cb5 100644 --- a/sei-db/ss/pebbledb/db.go +++ b/sei-db/ss/pebbledb/db.go @@ -59,6 +59,7 @@ var ( type Database struct { storage *pebble.DB + closed atomic.Bool // Set to true when Close() is called, checked by Prune() asyncWriteWG sync.WaitGroup config config.StateStoreConfig // Earliest version for db after pruning @@ -187,6 +188,9 @@ func New(dataDir string, config config.StateStoreConfig) (*Database, error) { } func (db *Database) Close() error { + // Mark as closed first to signal pruning to skip/return early + db.closed.Store(true) + // Stop background metrics collection if db.metricsCancel != nil { db.metricsCancel() @@ -201,6 +205,7 @@ func (db *Database) Close() error { _ = db.streamHandler.Close() db.streamHandler = nil } + var err error if db.storage != nil { err = db.storage.Close() @@ -634,8 +639,8 @@ func (db *Database) writeAsyncInBackground() { // it has been updated. This occurs when that module's keys are updated in between pruning runs, the node after is restarted. // This is not a large issue given the next time that module is updated, it will be properly pruned thereafter. func (db *Database) Prune(version int64) (_err error) { - // Defensive check: ensure database is not closed - if db.storage == nil { + // Check if database is closed + if db.closed.Load() { return errors.New("pebbledb: database is closed") } diff --git a/sei-db/ss/rocksdb/db.go b/sei-db/ss/rocksdb/db.go index e489f4e049..ff39c23341 100644 --- a/sei-db/ss/rocksdb/db.go +++ b/sei-db/ss/rocksdb/db.go @@ -49,6 +49,7 @@ type VersionedChangesets struct { type Database struct { storage *grocksdb.DB + closed atomic.Bool // Set to true when Close() is called, checked by Prune() config config.StateStoreConfig cfHandle *grocksdb.ColumnFamilyHandle @@ -293,8 +294,8 @@ func (db *Database) writeAsyncInBackground() { // lazy prune. Future compactions will honor the increased full_history_ts_low // and trim history when possible. func (db *Database) Prune(version int64) error { - // Defensive check: ensure database is not closed - if db.storage == nil { + // Check if database is closed + if db.closed.Load() { return fmt.Errorf("rocksdb: database is closed") } @@ -499,6 +500,9 @@ func (db *Database) WriteBlockRangeHash(storeKey string, beginBlockRange, endBlo } func (db *Database) Close() error { + // Mark as closed first to signal pruning goroutine to stop + db.closed.Store(true) + if db.streamHandler != nil { // Close the pending changes channel to signal the background goroutine to stop close(db.pendingChanges) @@ -509,6 +513,7 @@ func (db *Database) Close() error { // Only set to nil after background goroutine has finished db.streamHandler = nil } + db.cfHandle = nil if db.storage != nil { db.storage.Close() From 4eac440a8cd5b30fe59d3d0878f80d6f43da80c2 Mon Sep 17 00:00:00 2001 From: blindchaser Date: Wed, 4 Feb 2026 16:38:39 -0500 Subject: [PATCH 3/3] use CompareAndSwap for idempote close --- sei-db/ss/pebbledb/db.go | 5 ++++- sei-db/ss/rocksdb/db.go | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/sei-db/ss/pebbledb/db.go b/sei-db/ss/pebbledb/db.go index a845d95cb5..88f2657e2d 100644 --- a/sei-db/ss/pebbledb/db.go +++ b/sei-db/ss/pebbledb/db.go @@ -189,7 +189,10 @@ func New(dataDir string, config config.StateStoreConfig) (*Database, error) { func (db *Database) Close() error { // Mark as closed first to signal pruning to skip/return early - db.closed.Store(true) + // Use CompareAndSwap to ensure Close is idempotent + if !db.closed.CompareAndSwap(false, true) { + return nil + } // Stop background metrics collection if db.metricsCancel != nil { diff --git a/sei-db/ss/rocksdb/db.go b/sei-db/ss/rocksdb/db.go index ff39c23341..8ac6bfcae6 100644 --- a/sei-db/ss/rocksdb/db.go +++ b/sei-db/ss/rocksdb/db.go @@ -501,7 +501,10 @@ func (db *Database) WriteBlockRangeHash(storeKey string, beginBlockRange, endBlo func (db *Database) Close() error { // Mark as closed first to signal pruning goroutine to stop - db.closed.Store(true) + // Use CompareAndSwap to ensure Close is idempotent + if !db.closed.CompareAndSwap(false, true) { + return nil + } if db.streamHandler != nil { // Close the pending changes channel to signal the background goroutine to stop