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

Fix P-chain Shutdown deadlock #2686

Merged
merged 1 commit into from
Jan 31, 2024
Merged

Conversation

StephenButtolph
Copy link
Contributor

@StephenButtolph StephenButtolph commented Jan 31, 2024

Why this should be merged

Fixes deadlock observed here.

Currently it's possible that:

  1. Shutdown is called with the context lock held
  2. Gossip is called
  3. Inside Gossip the validator set needs to be recalculated
  4. Recalculating the validator set blocks until it can grab the context lock
  5. Shutdown blocks until Gossip returns

How this works

The options here are:

  1. Release the lock in Shutdown prior to waiting and then re-grabbing it
  2. Not blocking on Gossip in Shutdown.

This PR takes approach 2. Technically this could cause unexpected warnings to be logged... I felt like this was better than releasing the context lock in Shutdown. Hopefully all of this will be cleaned up once we push the context lock into the VMs.

How this was tested

  • CI

@StephenButtolph StephenButtolph added the bug Something isn't working label Jan 31, 2024
@StephenButtolph StephenButtolph added this to the v1.10.20 milestone Jan 31, 2024
@StephenButtolph StephenButtolph self-assigned this Jan 31, 2024
@StephenButtolph StephenButtolph added this pull request to the merge queue Jan 31, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 31, 2024
@StephenButtolph StephenButtolph added this pull request to the merge queue Jan 31, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jan 31, 2024
@StephenButtolph StephenButtolph merged commit e4fa404 into master Jan 31, 2024
17 checks passed
@StephenButtolph StephenButtolph deleted the fix-p-chain-shutdown-deadlock branch January 31, 2024 20:59
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
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants