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/downloader: fix possible data race by inconsistent field protection #20690

Merged
merged 1 commit into from
Feb 25, 2020
Merged

eth/downloader: fix possible data race by inconsistent field protection #20690

merged 1 commit into from
Feb 25, 2020

Conversation

BurtonQin
Copy link
Contributor

Fixed inconsistency and also potential data race in eth/downloader/queue.go:
e.g. In eth/downloader/queue.go:
q.receiptPendPool is read/written 10 times; 9 out of 10 times it is protected by q.lock.Lock(); 1 out of 10 times it is read without a Lock, which is in func CancelReceipts() on L579.
A data race may happen when CancelReceipts() and other func like Revoke() are called in parallel.
The fix is to move lock from func cancel() to its caller CancelReceipts().
The other bugs and fixes are similar.
Note that there is an FP for eth/downloader/peer.go in PR 20650. I remove the changes in peer.go and start a new PR here.

Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@karalabe karalabe added this to the 1.9.12 milestone Feb 25, 2020
@karalabe karalabe merged commit 2a5ed1a into ethereum:master Feb 25, 2020
kjx98 pushed a commit to kjx98/go-ethereum that referenced this pull request Mar 17, 2020
enriquefynn pushed a commit to enriquefynn/go-ethereum that referenced this pull request Mar 10, 2021
gzliudan pushed a commit to gzliudan/XDPoSChain that referenced this pull request Jun 27, 2024
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