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 transaction announce/broadcast goroutine leak #20762

Merged
merged 1 commit into from
Mar 14, 2020

Conversation

karalabe
Copy link
Member

The fail channels in the transaction announcement and broadcast loops were blocking, which meant that any announcement started must be allowed to finish before exiting the broadcaster. This was not the case as a peer drop concurrently closed the peer's termination channel and aborted the send. Depending on the ordering of the event, the outer broadcaster method might have reacted to the p.term closure faster, never reading the failure from fail.

One way to solve it would be to track any pending requests and only return from the outer method when the goroutine requests finish. This PR takes a simpler approach by converting the fail from a blocking channel to a notification one. This way if the sending goroutine is left dangling, it will just write into a noop channel.

@karalabe karalabe added this to the 1.9.12 milestone Mar 13, 2020
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

@karalabe karalabe merged commit 68b4b74 into ethereum:master Mar 14, 2020
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.

2 participants