Skip to content

Fix a deadlock in miner.rs #1968

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

Merged
merged 7 commits into from
Jun 3, 2020
Merged

Conversation

majecty
Copy link
Contributor

@majecty majecty commented May 21, 2020

There is a deadlock between immune_users and mem_pool.

A client worker thread locks immune_users first and mem_pool second in
prepare_block function.

An HTTP thread locks mem_pool first in import_own_transactions function
and locks immune_users in add_transactions_to_pool function.

It seems that this deadlock is added when we add immune_users lock. We
need to review the lock of immune_users.

@majecty majecty added the bug Something isn't working label May 21, 2020
@majecty majecty requested review from sgkim126 and HoOngEe May 21, 2020 10:10
@HoOngEe
Copy link
Contributor

HoOngEe commented May 21, 2020

How about just moving the line let immune_users = self.immune_users.read(); to the 565th line ?

Copy link
Contributor

@sgkim126 sgkim126 left a comment

Choose a reason for hiding this comment

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

Dropping the lock guard of immune_users will not help to fix the deadlock. Since the thread will stop when requiring to lock mem_pool. How about locking mem_pool at the 531st line. To make code always lock mem_pool first?

@majecty
Copy link
Contributor Author

majecty commented May 26, 2020

@HoOngEe @sgkim126 I agree with Seulgi. Locking mem_pool first will be better.

@majecty majecty removed the request for review from HoOngEe June 3, 2020 05:52
@majecty majecty marked this pull request as draft June 3, 2020 05:52
majecty added 7 commits June 3, 2020 15:54
NextMandatoryReseal limits the lifetime of the inner lock.
NextAllowedReseal limits the lifetime of the inner lock.
Params limits the lifetime of the inner lock.
SealingBlockLastRequest limits the lifetime of the inner lock.
Nrtofiers limits the lifetime of the inner lock.
Users limits the lifetime of the inner lock.
@majecty majecty force-pushed the f/hotfix-deadlock branch from bfbfce1 to a9e9700 Compare June 3, 2020 07:00
@majecty majecty changed the title Hotfix a deadlock Fix a deadlock in miner.rs Jun 3, 2020
@majecty majecty requested a review from sgkim126 June 3, 2020 07:01
@majecty majecty marked this pull request as ready for review June 3, 2020 07:02
@majecty majecty merged commit 65a0072 into CodeChain-io:rc-2.2.x Jun 3, 2020
@majecty majecty deleted the f/hotfix-deadlock branch June 3, 2020 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants