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 deadlock #5036

Merged
merged 9 commits into from
Aug 7, 2024
Merged

Fix deadlock #5036

merged 9 commits into from
Aug 7, 2024

Conversation

obycode
Copy link
Contributor

@obycode obycode commented Aug 1, 2024

Ensure that the chainstate db lock is not held when attempting to lock the sortition db to avoid deadlock.

@obycode obycode changed the base branch from master to develop August 1, 2024 19:34
@obycode
fix: release the chainstate db before locking the sortition db
8a7768d 
This ordering needs to be maintained to avoid deadlock.
@obycode
chore: move set_stacks_block_accepted later
e65ad15 
Also, `commit` before announcing the block via the dispatcher.
@obycode
test(wip): add test to verify deadlock in process_next_nakamoto_block
c973bea
@obycode
test: finish deadlock test
ae946dd
@obycode
test: insert stall to ensure proper timing of test
b557827 
With this change in place, `process_next_nakamoto_block_deadlock` will
timeout without the fix, and run successfully with the fix in place.
@obycode
Merge branch 'develop' into fix/deadlock
8185911
@obycode obycode marked this pull request as ready for review August 4, 2024 16:10
@obycode obycode requested a review from a team as a code owner August 4, 2024 16:10
@obycode
Copy link
Contributor Author

obycode commented Aug 4, 2024

The new test, process_next_nakamoto_block_deadlock hangs without this fix and passes with it.

@obycode obycode requested review from jcnelson and kantai August 4, 2024 16:11
jferrant
jferrant previously approved these changes Aug 5, 2024
View reviewed changes
kantai
kantai reviewed Aug 6, 2024
View reviewed changes
Copy link
Member

@kantai kantai left a comment

Choose a reason for hiding this comment

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

LGTM, just need to patch the build flags to get the non-test build to compile.

@obycode obycode enabled auto-merge August 7, 2024 12:10
@obycode obycode added this pull request to the merge queue Aug 7, 2024
Merged via the queue into develop with commit 8f8fe8d Aug 7, 2024
1 check passed
@blockstack-devops
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@stacks-network stacks-network locked as resolved and limited conversation to collaborators Oct 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kantai kantai kantai approved these changes

@jcnelson jcnelson jcnelson approved these changes

@jferrant jferrant jferrant left review comments

Assignees
No one assigned
Labels
locked
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@obycode @blockstack-devops @jcnelson @kantai @jferrant