NightOwl888 commented on code in PR #938: URL: https://github.com/apache/lucenenet/pull/938#discussion_r1597773408
########## src/Lucene.Net/Support/ConcurrentHashSet.cs: ########## @@ -753,7 +753,16 @@ private void CopyToItems(T[] array, int index) public void ExceptWith(IEnumerable<T> other) { - throw new NotImplementedException(); + if (ReferenceEquals(this, other)) Review Comment: My concern is less about whether this will succeed with FSync, and more about whether the `ConcurrentHashSet<T>` matches the behavior in Java, as it may be eventually cleaned up, moved to J2N, and have a full suite of Harmony + dotnet/runtime tests added to confirm it works as expected. Synchronizing around the entire set is what is done in Java. They use a wrapper class that locks access to the entire set. But IMO, we are better off emulating the `ConcurrentDictionary` in .NET, which uses locked "buckets". The whole set can be locked by locking all of the buckets. So, like what was done on `UnionWith`, if we are calling into existing code, we should separate the internal operations from the locking code so we can call into the business logic under different locking conditions. In this case, we should be locking all of the buckets at the beginning of the method call and then calling `InternalRemove` (which does no locking). > I don't think there's any amount of concurrent-izing this hashset that avoids these potential race conditions. The only proper solution would be to synchronize around the entire hashset, at which point we might as well just use the BCL non-concurrent HashSet instead. Let me know what you think. I guess that is also an option. The `ConcurrentSet` is just such a wrapper that locks the entire set as was done in Java. It can be used by calling `new HashSet<T>().AsConcurrent()`. But, it mostly only exists because of other JDK concerns such as structural equality, structural formatting, and because these `ISet<T>` operations are fully implemented. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@lucenenet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org