NightOwl888 commented on code in PR #938:
URL: https://github.com/apache/lucenenet/pull/938#discussion_r1597830128
##########
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))
+ {
+ Clear();
+ return;
+ }
+
+ foreach (var item in other)
+ {
+ TryRemove(item);
Review Comment:
> I'm now suspicious of other uses of this type btw.
Well, you should be :blush:. Most of the concurrency problems we found were
due to incorrect lock usage, but these issues are difficult to track down. I
suspect that #935 is due to a missing lock somewhere, but it could also be some
sort of timing issue. There are 3 locking scenarios that are most likely:
- A collection type is not being locked in a similar enough way to Java to
meet our requirements. That is, an issue with the design of the collection that
was chosen to replace the one in Lucene.
- A collection is being used as a lock object, but the collection is not
implemented in a way where it is sharing the lock internally. To fix these,
some collections were set up to use the [`SyncRoot`
property](https://learn.microsoft.com/en-us/dotnet/api/system.collections.icollection.syncroot?view=net-8.0),
but I am not sure whether that is the best solution.
- A type was changed from `lock (this)` to `lock (syncLock)` (or in our case
`UninterruptableMonitor.Enter(syncLock)`) and the lock is not being shared
everywhere it was in Lucene. In particular, we need to look out for inherited
types that should be sharing the same lock as its base class and external code
that uses the object as a lock object.
Fortunately, #935 is the only known concurrency bug in the core assembly and
fixing it is not a breaking change so it is not blocking the release.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]