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

Suspicious invocation of the 'IEnumerable<T>.Union' method #5862

Closed
VasilievSerg opened this issue Apr 21, 2022 · 2 comments · Fixed by #5863
Closed

Suspicious invocation of the 'IEnumerable<T>.Union' method #5862

VasilievSerg opened this issue Apr 21, 2022 · 2 comments · Fixed by #5863

Comments

@VasilievSerg
Copy link

Hi,
I found the suspicious code via the PVS-Studio analyzer.

Here is the code:

...
case ShardId shardId:
  _shards.Add(shardId); // <=
  return true;
case SnapshotOffer offer when (offer.Snapshot is ShardCoordinator.CoordinatorState state):
  _shards.UnionWith(state.Shards.Keys.Union(state.UnallocatedShards)); // <=
  return true;
case SnapshotOffer offer when (offer.Snapshot is State state):
  _shards.Union(state.Shards); // <=
...

Here is the link to the sources.

Several methods were called for the _shards variable. The last invocation looks suspicious.

Add and UnionWith are methods of the HashSet<T> type. They change the _shards object state. But Union is the extension method which does not change the _shards state. The result of this invocation is not used as well.

@VasilievSerg VasilievSerg changed the title Incorrect invocation of the 'IEnumerable<T>.Union' method Suspicious invocation of the 'IEnumerable<T>.Union' method Apr 21, 2022
@Aaronontheweb Aaronontheweb added this to the 1.5.0 milestone Apr 21, 2022
@Aaronontheweb
Copy link
Member

Ah, so this is on the brand new 1.5 code - interesting, we'll take a look.

@Aaronontheweb
Copy link
Member

cc @zbynek001 - is this something I overlooked during the PR review on the sharding storage update?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants