Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix crash on invalid snapshot #874

Merged
merged 2 commits into from
Jun 27, 2024
Merged

fix crash on invalid snapshot #874

merged 2 commits into from
Jun 27, 2024

Conversation

brennanjl
Copy link
Collaborator

I ran into an issue where I stopped by node at just the right time where it was able to get bricked and become unrecoverable without manual intervention. While creating a snapshot, my node stopped, and was then unable to restart. It would crash on restart due to a nil pointer dereference. This PR fixes this.

jchappelow
jchappelow previously approved these changes Jun 26, 2024
Copy link
Member

@jchappelow jchappelow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK but we should Close those files that we Open IMO.

@jchappelow
Copy link
Member

LoadSnapshotChunk has the Open problem too. That method I believe is called on demand by peers, so DOS

@jchappelow jchappelow added this to the v0.8.3 milestone Jun 26, 2024
@charithabandi
Copy link
Contributor

Looks good!

@brennanjl brennanjl merged commit 77b08dd into main Jun 27, 2024
2 checks passed
@brennanjl brennanjl deleted the fix-snapshot-crash branch June 27, 2024 16:20
brennanjl added a commit that referenced this pull request Jun 27, 2024
* fix crash on invalid snapshot

* smarter snapshot chunk loading

---------

Co-authored-by: Jonathan Chappelow <[email protected]>
brennanjl added a commit that referenced this pull request Jun 27, 2024
* fix crash on invalid snapshot

* smarter snapshot chunk loading

---------

Co-authored-by: Jonathan Chappelow <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants