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(core/listener): wait listener to shutdown before exit #3775

Merged
merged 2 commits into from
Sep 25, 2024

Conversation

walldiss
Copy link
Member

@walldiss walldiss commented Sep 25, 2024

Previously we observed panic with leveldb: closed in integration tests. It was caused by core.listener accessing already closed core block generator. The reason for it could be leaked listener subscriber routine after node is closed, that was trying to access generator.

The PR introduces graceful shutdown for listener, that will wait for spawned routines to stop before exiting.

@walldiss walldiss added the kind:fix Attached to bug-fixing PRs label Sep 25, 2024
@walldiss walldiss self-assigned this Sep 25, 2024
@codecov-commenter
Copy link

codecov-commenter commented Sep 25, 2024

Codecov Report

Attention: Patch coverage is 61.53846% with 5 lines in your changes missing coverage. Please review.

Project coverage is 46.50%. Comparing base (2469e7a) to head (8df8873).
Report is 311 commits behind head on main.

Files with missing lines Patch % Lines
core/listener.go 61.53% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3775      +/-   ##
==========================================
+ Coverage   44.83%   46.50%   +1.67%     
==========================================
  Files         265      314      +49     
  Lines       14620    17999    +3379     
==========================================
+ Hits         6555     8371    +1816     
- Misses       7313     8609    +1296     
- Partials      752     1019     +267     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

Please explain how it fixes the flake either in the comment or PR description for future ref

renaynay
renaynay previously approved these changes Sep 25, 2024
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

Nit: Set chan to nil when done ? And make chan on start ?

@walldiss walldiss merged commit 98a127b into celestiaorg:main Sep 25, 2024
32 checks passed
@walldiss walldiss deleted the wait-listner-shutdown branch September 25, 2024 14:45
walldiss added a commit to walldiss/celestia-node that referenced this pull request Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:fix Attached to bug-fixing PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants