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

whisper: bloom filter refactoring #16046

Merged
merged 2 commits into from
Feb 9, 2018
Merged

Conversation

gluk256
Copy link
Contributor

@gluk256 gluk256 commented Feb 9, 2018

No description provided.

Copy link
Member

@gballet gballet left a comment

Choose a reason for hiding this comment

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

Have a look at a potential concurrency issue when getting the bloom filter.

peer.bloomFilter = bloom
peer.fullNode = isFullNode(bloom)
if peer.fullNode {
if peer.bloomFilter == nil {
Copy link
Member

Choose a reason for hiding this comment

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

I see a problem here, because I see that you are initializing it to all 1s when you set it. That means that, at some point, there could be someone trying to read the bloom filter before someone has initialized it. So I believe there should be a getBloomFilter as well.

@karalabe karalabe added this to the 1.8.0 milestone Feb 9, 2018
@karalabe karalabe merged commit 42628ba into ethereum:master Feb 9, 2018
prestonvanloon pushed a commit to prestonvanloon/go-ethereum that referenced this pull request Apr 2, 2018
* whisper: bloom filter refactoring

* whisper: fixed full node
mariameda pushed a commit to NiluPlatform/go-nilu that referenced this pull request Aug 23, 2018
* whisper: bloom filter refactoring

* whisper: fixed full node
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