-
Notifications
You must be signed in to change notification settings - Fork 670
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 Concurrency Bug In CommitToParent #1320
Conversation
@@ -222,10 +222,8 @@ func (t *trieView) calculateNodeIDs(ctx context.Context) error { | |||
|
|||
// ensure that the view under this one is up-to-date before potentially pulling in nodes from it | |||
// getting the Merkle root forces any unupdated nodes to recalculate their ids | |||
if t.parentTrie != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
detects as race and no longer needed. Hold over from when views could have no parent view.
Co-authored-by: Dan Laine <daniel.laine@avalabs.org>
…alanchego into FixCommitToParentBug
t.db.commitLock.Lock() | ||
defer t.db.commitLock.Unlock() | ||
} | ||
t.db.commitLock.Lock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevents multiple commits from occurring at the same time until I can figure out a way to manage the safety of this.
…alanchego into FixCommitToParentBug
Co-authored-by: Dan Laine <daniel.laine@avalabs.org>
CommitToParent can happen concurrently so it may cause the parent of a view to change during its own attempt to commit to its parent(see the new unit tests). This isn't an issue for CommitToDB as it uses the commit lock.