diff --git a/src/Lucene.Net/Index/IndexReader.cs b/src/Lucene.Net/Index/IndexReader.cs index d97842f02c..c75c80b5d9 100644 --- a/src/Lucene.Net/Index/IndexReader.cs +++ b/src/Lucene.Net/Index/IndexReader.cs @@ -100,7 +100,14 @@ public interface IReaderClosedListener private readonly ISet readerClosedListeners = new JCG.LinkedHashSet().AsConcurrent(); - private readonly ISet> parentReaders = new ConcurrentHashSet>(); +#if FEATURE_CONDITIONALWEAKTABLE_ENUMERATOR + private readonly ConditionalWeakTable parentReaders = new ConditionalWeakTable(); +#else + private readonly WeakDictionary parentReaders = new WeakDictionary(); +#endif + // LUCENENET specific - since neither WeakDictionary nor ConditionalWeakTable synchronize + // on the enumerator, we need to do external synchronization to make them threadsafe. + private readonly object parentReadersLock = new object(); /// /// Expert: adds a . The @@ -136,7 +143,14 @@ public void RemoveReaderClosedListener(IReaderClosedListener listener) public void RegisterParentReader(IndexReader reader) { EnsureOpen(); - parentReaders.Add(new IdentityWeakReference(reader)); + // LUCENENET specific - since neither WeakDictionary nor ConditionalWeakTable synchronize + // on the enumerator, we need to do external synchronization to make them threadsafe. + lock (parentReadersLock) + // LUCENENET: Since there is a set Add operation (unique) in Lucene, the equivalent + // operation in .NET is AddOrUpdate, which effectively does nothing if the key exists. + // Null is passed as a value, since it is not used anyway and .NET doesn't have a boolean + // reference type. + parentReaders.AddOrUpdate(key: reader, value: null); } private void NotifyReaderClosedListeners(Exception th) @@ -167,15 +181,18 @@ private void NotifyReaderClosedListeners(Exception th) private void ReportCloseToParentReaders() { - lock (parentReaders) // LUCENENET: This does not actually synchronize the set, it only ensures this method can only be entered by 1 thread + // LUCENENET specific - since neither WeakDictionary nor ConditionalWeakTable synchronize + // on the enumerator, we need to do external synchronization to make them threadsafe. + lock (parentReadersLock) { - foreach (IdentityWeakReference parent in parentReaders) + foreach (var kvp in parentReaders) { - //Using weak references - IndexReader target = parent.Target; + IndexReader target = kvp.Key; + // LUCENENET: This probably can't happen, but we are being defensive to avoid exceptions if (target != null) { + //Using weak references target.closedByChild = true; // cross memory barrier by a fake write: target.refCount.AddAndGet(0); diff --git a/src/Lucene.Net/Support/IdentityWeakReference.cs b/src/Lucene.Net/Support/IdentityWeakReference.cs deleted file mode 100644 index dac12b80c2..0000000000 --- a/src/Lucene.Net/Support/IdentityWeakReference.cs +++ /dev/null @@ -1,59 +0,0 @@ -using System; -using System.Runtime.CompilerServices; - -namespace Lucene.Net.Support -{ - /* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - - internal class IdentityWeakReference : WeakReference - where T : class - { - private readonly int hash; - private static readonly object NULL = new object(); - - public IdentityWeakReference(T target) - : base(target ?? NULL) - { - hash = RuntimeHelpers.GetHashCode(target); - } - - public override int GetHashCode() - { - return hash; - } - - public override bool Equals(object o) - { - if (ReferenceEquals(this, o)) - { - return true; - } - if (o is IdentityWeakReference iwr && ReferenceEquals(this.Target, iwr.Target)) - { - return true; - } - return false; - } - - public new T Target - { // note: if this.NULL is the target, it will not cast to T, so the "as" will return null as we would expect. - get => base.Target as T; - set => base.Target = value; - } - } -} \ No newline at end of file diff --git a/src/Lucene.Net/Support/WeakDictionary.cs b/src/Lucene.Net/Support/WeakDictionary.cs index 12673adbbb..bc2667bc27 100644 --- a/src/Lucene.Net/Support/WeakDictionary.cs +++ b/src/Lucene.Net/Support/WeakDictionary.cs @@ -19,12 +19,11 @@ * */ - #if !FEATURE_CONDITIONALWEAKTABLE_ENUMERATOR using System; using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; -using System.Linq; using JCG = J2N.Collections.Generic; namespace Lucene.Net.Support @@ -51,7 +50,9 @@ public WeakDictionary(IEnumerable> otherDictionary) private WeakDictionary(int initialCapacity, IEnumerable> otherDict) { - _hm = new JCG.Dictionary, TValue>(initialCapacity); + // LUCENENET specific - using ConcurrentDictionary here not because we need concurrency, but because + // it allows removing items while iterating forward. + _hm = new ConcurrentDictionary, TValue>(concurrencyLevel: 10, capacity: initialCapacity); foreach (var kvp in otherDict) { _hm.Add(new WeakKey(kvp.Key), kvp.Value); @@ -65,12 +66,11 @@ private WeakDictionary(int initialCapacity, IEnumerable, TValue>(_hm.Count); - foreach (var entry in _hm.Where(x => x.Key != null && x.Key.IsAlive)) + foreach (var kvp in _hm) { - newHm.Add(entry.Key, entry.Value); + if (!kvp.Key.TryGetTarget(out TKey _)) + _hm.Remove(kvp.Key); } - _hm = newHm; } private void CleanIfNeeded() @@ -85,9 +85,12 @@ private void CleanIfNeeded() public IEnumerator> GetEnumerator() { - foreach (var kvp in _hm.Where(x => x.Key.IsAlive)) + foreach (var kvp in _hm) { - yield return new KeyValuePair(kvp.Key.Target, kvp.Value); + if (kvp.Key.TryGetTarget(out TKey key)) + { + yield return new KeyValuePair(key, kvp.Value); + } } } @@ -185,7 +188,7 @@ void ICollection>.CopyTo(KeyValuePair[] throw new NotSupportedException(); } -#region KeyCollection + #region Nested Class: KeyCollection private class KeyCollection : ICollection { @@ -200,7 +203,8 @@ public IEnumerator GetEnumerator() { foreach (var key in _internalDict.Keys) { - yield return key.Target; + if (key.TryGetTarget(out TKey target)) + yield return target; } } @@ -243,7 +247,7 @@ bool ICollection.Remove(TKey item) #endregion Explicit Interface Definitions } -#endregion KeyCollection + #endregion Nested Class: KeyCollection /// /// A weak reference wrapper for the hashtable keys. Whenever a key\value pair @@ -252,7 +256,7 @@ bool ICollection.Remove(TKey item) /// private class WeakKey where T : class { - private readonly WeakReference reference; + private readonly WeakReference reference; private readonly int hashCode; public WeakKey(T key) @@ -260,8 +264,8 @@ public WeakKey(T key) if (key is null) throw new ArgumentNullException(nameof(key)); - hashCode = key.GetHashCode(); - reference = new WeakReference(key); + hashCode = key is null ? 0 : key.GetHashCode(); + reference = new WeakReference(key); } public override int GetHashCode() @@ -271,25 +275,20 @@ public override int GetHashCode() public override bool Equals(object obj) { - if (!reference.IsAlive || obj is null) return false; - - if (object.ReferenceEquals(this, obj)) - { - return true; - } - + if (obj is null) return false; if (obj is WeakKey other) { - var referenceTarget = reference.Target; // Careful: can be null in the mean time... - return referenceTarget != null && referenceTarget.Equals(other.Target); + bool gotThis = this.TryGetTarget(out T thisTarget), gotOther = other.TryGetTarget(out T otherTarget); + if (gotThis && gotOther && thisTarget.Equals(otherTarget)) + return true; + else if (gotThis == false && gotOther == false) + return true; } return false; } - public T Target => (T)reference.Target; - - public bool IsAlive => reference.IsAlive; + public bool TryGetTarget(out T target) => reference.TryGetTarget(out target); } } }