Skip to content
This repository was archived by the owner on Aug 19, 2024. It is now read-only.

fixed possible datarace for downloader queue#749

Merged
jeongkyun-oh merged 2 commits intoklaytn:devfrom
jeongkyun-oh:KLT-360-fix-cancel-datarace
Nov 19, 2020
Merged

fixed possible datarace for downloader queue#749
jeongkyun-oh merged 2 commits intoklaytn:devfrom
jeongkyun-oh:KLT-360-fix-cancel-datarace

Conversation

@jeongkyun-oh
Copy link
Copy Markdown
Contributor

@jeongkyun-oh jeongkyun-oh commented Nov 18, 2020

Proposed changes

Types of changes

Please put an x in the boxes related to your change.

  • Bugfix
  • New feature or enhancement
  • Others

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING GUIDELINES doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes ($ make test)
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

Comment thread datasync/downloader/queue.go Outdated
Co-authored-by: Yumiel (yoomee1313) <yumiel.ko@groundx.xyz>
@aidan-kwon
Copy link
Copy Markdown
Member

@jeongkyun-oh I cannot find any relation with ethereum/go-ethereum#21201. Indeed, the old code looks better for me.

@jeongkyun-oh
Copy link
Copy Markdown
Contributor Author

jeongkyun-oh commented Nov 19, 2020

@aidan-kwon Sorry, it is related to 20690. The lock is acquired in each cancel function, which may cause data-race if other threads access the pending pool. The point is that the lock is acquired after reading pendPool which may be written by other goroutines.

@jeongkyun-oh jeongkyun-oh merged commit b60e9c6 into klaytn:dev Nov 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants