Skip to content

R4R: Add missing iterator close#3387

Merged
cwgoes merged 1 commit intocosmos:developfrom
HaoyangLiu:irisnet/add-missing-iterator-close
Jan 25, 2019
Merged

R4R: Add missing iterator close#3387
cwgoes merged 1 commit intocosmos:developfrom
HaoyangLiu:irisnet/add-missing-iterator-close

Conversation

@HaoyangLiu
Copy link
Copy Markdown

No description provided.

// Iterate over validators, highest power to lowest.
iterator := sdk.KVStoreReversePrefixIterator(store, ValidatorsByPowerIndexKey)
defer iterator.Close()
count := 0
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The iterator will be created in every endBlocker. This missing may cause huge memory leak.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yikes
\

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 25, 2019

Codecov Report

Merging #3387 into develop will increase coverage by <.01%.
The diff coverage is 61.53%.

@@             Coverage Diff             @@
##           develop    #3387      +/-   ##
===========================================
+ Coverage    59.32%   59.32%   +<.01%     
===========================================
  Files          131      131              
  Lines         9855     9862       +7     
===========================================
+ Hits          5846     5851       +5     
- Misses        3636     3638       +2     
  Partials       373      373

Copy link
Copy Markdown
Contributor

@rigelrozanski rigelrozanski left a comment

Choose a reason for hiding this comment

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

It would be good to build a CI utility to check for missing iterator close statements

@cwgoes cwgoes merged commit afb04b1 into cosmos:develop Jan 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants