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

Change checkpoint reorg in update checkpoint #916

Merged

Conversation

@Doy-lee
Copy link
Collaborator

Doy-lee commented Oct 28, 2019

Removing popping blocks in update_checkpoint which is often called by the networking thread when receiving enough votes to make a new checkpoint sometimes causing the daemon to lock up.

Now, if we receive a checkpoint that is conflicting with our blockchain, we remain on the alt-chain until we receive an additional block on the correct chain. Upon receiving the new block, we re-evaluate each block in the alt chain and the number of checkpoints that match for either chain, and only then do we switch to the alt chain (using the main block processing thread, thereby avoiding locking problems in update_checkpoint).

This will bump the patch to version 3.

  • Also small bug fix I found making daemons keep alt chains for longer than they should.

@jagerman

@Doy-lee Doy-lee force-pushed the Doy-lee:KillCheckpointReorgInUpdateCheckpoint branch 2 times, most recently from 7ec7037 to aa14848 Oct 28, 2019
Copy link

jagerman left a comment

Looks good for now.

I think this deadlock is coming down to Monero's locking being not well implemented: everything just takes out an exclusive lock, but there are multiple locks that require a specific order; and the locks are all recursive so that the code can be sloppy just take out multiple overlapping locks. But if locks get out of order (because of locks taken out somewhere up or down the call stack) the result is a deadlock, and locks are taken out in enough different places that it's almost impossible to know whether the code is deadlock free.

That said, as much as I want to overhaul the locking, it isn't a change for 5.x.

Doy-lee added 2 commits Oct 28, 2019
Hold onto the invalid chain until you receive another alt block and
cause a re-org on the block processing thread.
@Doy-lee Doy-lee force-pushed the Doy-lee:KillCheckpointReorgInUpdateCheckpoint branch from aa14848 to 4c8d9e8 Oct 28, 2019
@Doy-lee Doy-lee merged commit 7b73841 into loki-project:master Oct 28, 2019
1 check was pending
1 check was pending
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.