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

eth: fix potential hang in waitSnapExtension #28744

Merged
merged 5 commits into from
Jan 15, 2024
Merged

eth: fix potential hang in waitSnapExtension #28744

merged 5 commits into from
Jan 15, 2024

Conversation

niuxiaojie81
Copy link
Contributor

When geth is shut down gracefully, the running eth protocol blocks in waitSnapExtension

Steps to reproduce:

  1. Establish the eth protocol and run the runEthPeer function. At this time, the incHandlers function returns true, waits for snap extension, and blocks in the waitSnapExtension.
  2. Close geth normally.
  3. Establish the snap protocol and run the runSnapExtension function. At this time, the incHandlers function returns false and will not enter the registerSnapExtension.
  4. The runEthPeer function has been blocked in waitSnapExtension, and the ETH handler cannot exit normally.

@niuxiaojie81
Copy link
Contributor Author

The fix refers to this PR, but the blocking scenario is completely different from it

@fjl
Copy link
Contributor

fjl commented Jan 9, 2024

I appreciate the clear description. The fix seems weird though, and could be improved to not use a timeout. How about we just add a closed channel which we close() when the peerSet is shut down, and then perfom select on this channel and wait in waitSnapExtension?

@niuxiaojie81
Copy link
Contributor Author

@fjl Yes, your modification suggestion is more reasonable. I have updated it. Please review it.

@fjl fjl added this to the 1.13.10 milestone Jan 10, 2024
@holiman holiman modified the milestones: 1.13.10, 1.13.11 Jan 12, 2024
eth/peerset.go Outdated Show resolved Hide resolved
eth/peerset.go Outdated Show resolved Hide resolved
Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM

@fjl fjl changed the title eth: avoid blocking when geth is closed, fix hang in waitSnapExtension eth: fix potential hang in waitSnapExtension Jan 15, 2024
@fjl fjl merged commit 18e154e into ethereum:master Jan 15, 2024
1 of 2 checks passed
Dergarcon pushed a commit to specialmechanisms/mev-geth-0x2mev that referenced this pull request Jan 31, 2024
This should fix a rare hang in waitSnapExtension during shutdown.
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