Skip to content

Commit

Permalink
fix potential mem-leak from hotstuff
Browse files Browse the repository at this point in the history
  • Loading branch information
zhangchiqing committed Oct 22, 2021
1 parent 38702f7 commit f6218fa
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
9 changes: 9 additions & 0 deletions consensus/hotstuff/voteaggregator/vote_aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,15 @@ func (va *VoteAggregator) StoreProposerVote(vote *model.Vote) bool {
return false
}
va.proposerVotes[vote.BlockID] = vote
// update viewToBlockIDSet
blockIDSet, exists := va.viewToBlockIDSet[vote.View]
if exists {
blockIDSet[vote.BlockID] = struct{}{}
} else {
blockIDSet = make(map[flow.Identifier]struct{})
blockIDSet[vote.BlockID] = struct{}{}
va.viewToBlockIDSet[vote.View] = blockIDSet
}
return true
}

Expand Down
21 changes: 21 additions & 0 deletions consensus/hotstuff/voteaggregator/vote_aggregator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,27 @@ func (as *AggregatorSuite) TestNonePruneAfterBlock() {
require.Equal(as.T(), 3, votingStatusLen)
}

// receive the block for view 2,3,4,5
// prune by view 5, should be all pruned
func (as *AggregatorSuite) TestPruneAll() {
pruneView := uint64(5)
for i := 2; i <= 5; i++ {
view := uint64(i)
bp := newMockBlock(as, view, as.participants[i].NodeID)
as.aggregator.StoreProposerVote(bp.ProposerVote())
}

require.Len(as.T(), as.aggregator.proposerVotes, 4)

as.aggregator.PruneByView(pruneView)
// proposerVotes should be all pruned, otherwise, there is memory leak
require.Len(as.T(), as.aggregator.proposerVotes, 0)
require.Len(as.T(), as.aggregator.blockIDToVotingStatus, 0)
require.Len(as.T(), as.aggregator.createdQC, 0)
require.Len(as.T(), as.aggregator.viewToVoteID, 0)
require.Len(as.T(), as.aggregator.viewToBlockIDSet, 0)
}

// RANDOM BEACON
// if there are 7 nodes, it requires 4 votes for random beacon,
// and receives the block from proposer who has 80% stake,
Expand Down

0 comments on commit f6218fa

Please sign in to comment.