Skip to content

Check storage is non-nil before attempting to close it#2659

Merged
masih merged 3 commits intomainfrom
masih/fix-nil-pointer-db-close
Jan 6, 2026
Merged

Check storage is non-nil before attempting to close it#2659
masih merged 3 commits intomainfrom
masih/fix-nil-pointer-db-close

Conversation

@masih
Copy link
Copy Markdown
Collaborator

@masih masih commented Jan 5, 2026

Check that the underlying storage object is non-nil before attempting to close pebble DB. Because, it is possible for the close to be called multiple times, which otherwise will result in panic.

The changes here make the multi-call to Close into noop.

Fixes STO-277

@masih masih marked this pull request as ready for review January 5, 2026 17:16
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 5, 2026

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedJan 6, 2026, 12:07 PM

@masih masih enabled auto-merge (squash) January 5, 2026 17:19
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 5, 2026

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.76%. Comparing base (d7d72d9) to head (878021e).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
sei-db/state_db/ss/test/storage_test_suite.go 0.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2659      +/-   ##
==========================================
- Coverage   46.34%   43.76%   -2.58%     
==========================================
  Files        1239     1906     +667     
  Lines      108460   159033   +50573     
==========================================
+ Hits        50262    69603   +19341     
- Misses      53595    83030   +29435     
- Partials     4603     6400    +1797     
Flag Coverage Δ
sei-chain 45.75% <66.66%> (+<0.01%) ⬆️
sei-cosmos 38.20% <ø> (?)
sei-db 69.13% <100.00%> (+0.06%) ⬆️
sei-tendermint 47.35% <ø> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
sei-db/db_engine/pebbledb/mvcc/db.go 64.16% <100.00%> (-0.20%) ⬇️
sei-db/db_engine/rocksdb/mvcc/db.go 58.80% <100.00%> (+0.16%) ⬆️
sei-db/state_db/ss/test/storage_test_suite.go 0.00% <0.00%> (ø)

... and 682 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Check that the underlying storage object is non-nil before attempting to close pebble DB. Because, it is possible for the close to be called multiple times, which otherwise will result in panic.

The changes here make the multi-call to `Close` into noop.

Fixes STO-277
@masih masih force-pushed the masih/fix-nil-pointer-db-close branch from 46d8cbf to b0c25eb Compare January 6, 2026 10:31
Make close idempotent since it is harmless to do so, and avoids unnecessary panics. It also adheres to the common behavior of IO package in Go SDK.
@masih masih disabled auto-merge January 6, 2026 12:05
@masih masih enabled auto-merge (squash) January 6, 2026 12:52
@masih masih merged commit 169e1fa into main Jan 6, 2026
40 of 42 checks passed
@masih masih deleted the masih/fix-nil-pointer-db-close branch January 6, 2026 13:12
github-actions bot pushed a commit that referenced this pull request Jan 6, 2026
Check that the underlying storage object is non-nil before attempting to
close pebble DB. Because, it is possible for the close to be called
multiple times, which otherwise will result in panic.

The changes here make the multi-call to `Close` into noop.

Fixes STO-277

(cherry picked from commit 169e1fa)
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 6, 2026

Successfully created backport PR for release/v6.3:

masih pushed a commit that referenced this pull request Jan 6, 2026
philipsu522 pushed a commit that referenced this pull request Jan 6, 2026
Check that the underlying storage object is non-nil before attempting to
close pebble DB. Because, it is possible for the close to be called
multiple times, which otherwise will result in panic.

The changes here make the multi-call to `Close` into noop.

Fixes STO-277
yzang2019 added a commit that referenced this pull request Jan 8, 2026
* main:
  feat: add generic KV interfaces + Pebble adapter (#2666)
  Make SSTORE chain param height-aware (#2667)
  fix: cosmos: protect coin denom regexp with a lock (#2660)
  Install CA certs on Ubuntu base image (#2658)
  Check storage is non-nil before attempting to close it (#2659)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants