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.

Sign up for GitHub

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

Jump to bottom

fix: orphan block pool deadlock #2074

Merged
merged 1 commit into from
May 18, 2020
Merged

fix: orphan block pool deadlock #2074

merged 1 commit into from
May 18, 2020

Conversation

quake
Copy link
Member

@quake quake commented May 13, 2020

resolved potential deadlock in OrphanBlockPool and a unit test which simulates deadlocking was added also.

@quake quake requested review from a team and zhangsoledad May 13, 2020 12:41
doitian
doitian approved these changes May 14, 2020
View reviewed changes
Copy link
Member

@doitian doitian left a comment

Choose a reason for hiding this comment

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

I'm OK with the fix. Just raise a question: how to avoid the similar bugs in the future?

@doitian
Copy link
Member

doitian commented May 14, 2020

I'm OK with the fix. Just raise a question: how to avoid the similar bugs in the future?

Since every function requires the lock of blocks first, maybe an internal structure as below will work.

internal: RwLock<(BlocksMap, RwLock<ParentsMap>)>,

@quake
Copy link
Member Author

quake commented May 15, 2020

I'm OK with the fix. Just raise a question: how to avoid the similar bugs in the future?

I think it can be detected by feature flag deadlock_detection, however it's not enabled in release build

@doitian doitian added the urgent Has an upcoming deadline label May 18, 2020
@doitian
Copy link
Member

doitian commented May 18, 2020

https://github.com/tokio-rs/loom may help here to reproduce the bug deterministically.

@doitian
Copy link
Member

doitian commented May 18, 2020

bors r=doitian,zhangsoledad

@bors
Copy link
Contributor

bors bot commented May 18, 2020

Build succeeded:

@bors bors bot merged commit 5ae1b8c into develop May 18, 2020
@bors bors bot deleted the quake/fix-orphan-block-pool-deadlock branch May 18, 2020 09:14
doitian pushed a commit that referenced this pull request May 21, 2020
@bors @quake
@xxuejie @doitian
Merge #2074 #2084
fa2263a 
2074: fix: orphan block pool deadlock r=doitian,zhangsoledad a=quake

resolved potential deadlock in `OrphanBlockPool`  and a unit test which simulates deadlocking was added also.

2084: feat: Expose methods so we can use CKB as a library r=doitian,yangby-cryptape a=xxuejie



Co-authored-by: quake <[email protected]>
Co-authored-by: Xuejie Xiao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhangsoledad zhangsoledad zhangsoledad approved these changes

@doitian doitian doitian approved these changes

Assignees
No one assigned
Labels
s:backport-needed urgent Has an upcoming deadline
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@quake @doitian @zhangsoledad