From 0dce38eba8cdcc53e8b7b214467c5c84bf743892 Mon Sep 17 00:00:00 2001 From: Shad Storhaug Date: Sat, 7 Nov 2020 03:15:28 +0700 Subject: [PATCH] Fixes CA1063: Implement IDisposable Properly (except for IndexWriter). Partially addresses #265. --- .../Facet/DistanceFacetsExample.cs | 2 +- .../Facet/RangeFacetsExample.cs | 2 +- src/Lucene.Net.Suggest/Spell/SpellChecker.cs | 33 +++++++----- .../CompressingTermVectorsReader.cs | 2 +- src/Lucene.Net/Codecs/DocValuesProducer.cs | 2 +- .../Lucene40/Lucene40StoredFieldsReader.cs | 2 +- .../Lucene40/Lucene40TermVectorsReader.cs | 2 +- .../Lucene45/Lucene45DocValuesConsumer.cs | 2 +- .../Lucene45/Lucene45DocValuesProducer.cs | 2 +- .../Search/ControlledRealTimeReopenThread.cs | 52 +++++++++++++------ src/Lucene.Net/Search/LiveFieldValues.cs | 23 +++++++- src/Lucene.Net/Search/ReferenceManager.cs | 27 ++++++---- .../Search/SearcherLifetimeManager.cs | 48 +++++++++++------ .../Store/OutputStreamDataOutput.cs | 24 ++++++++- 14 files changed, 155 insertions(+), 68 deletions(-) diff --git a/src/Lucene.Net.Demo/Facet/DistanceFacetsExample.cs b/src/Lucene.Net.Demo/Facet/DistanceFacetsExample.cs index d28c6cad61..f5e4ed527a 100644 --- a/src/Lucene.Net.Demo/Facet/DistanceFacetsExample.cs +++ b/src/Lucene.Net.Demo/Facet/DistanceFacetsExample.cs @@ -45,7 +45,7 @@ namespace Lucene.Net.Demo.Facet /// Shows simple usage of dynamic range faceting, using the /// expressions module to calculate distance. /// - public class DistanceFacetsExample : IDisposable + public sealed class DistanceFacetsExample : IDisposable { /// /// Using a constant for all functionality related to a specific index diff --git a/src/Lucene.Net.Demo/Facet/RangeFacetsExample.cs b/src/Lucene.Net.Demo/Facet/RangeFacetsExample.cs index 180c39b95a..a2cb930898 100644 --- a/src/Lucene.Net.Demo/Facet/RangeFacetsExample.cs +++ b/src/Lucene.Net.Demo/Facet/RangeFacetsExample.cs @@ -35,7 +35,7 @@ namespace Lucene.Net.Demo.Facet /// /// Shows simple usage of dynamic range faceting. /// - public class RangeFacetsExample : IDisposable + public sealed class RangeFacetsExample : IDisposable { /// /// Using a constant for all functionality related to a specific index diff --git a/src/Lucene.Net.Suggest/Spell/SpellChecker.cs b/src/Lucene.Net.Suggest/Spell/SpellChecker.cs index 2fa8cbb2a0..6dc0a40adc 100644 --- a/src/Lucene.Net.Suggest/Spell/SpellChecker.cs +++ b/src/Lucene.Net.Suggest/Spell/SpellChecker.cs @@ -46,7 +46,6 @@ namespace Lucene.Net.Search.Spell /// public class SpellChecker : IDisposable { - /// /// The default minimum score to use, if not specified by setting /// or overriding with . @@ -650,25 +649,36 @@ private void EnsureOpen() { if (disposed) { - throw new ObjectDisposedException(this.GetType().FullName, "Spellchecker has been closed"); + throw new ObjectDisposedException(this.GetType().FullName, "Spellchecker has been disposed."); } } /// - /// Dispose the underlying IndexSearcher used by this SpellChecker + /// Dispose the underlying used by this . /// if the close operation causes an /// if the is already disposed public void Dispose() { - if (!disposed) + Dispose(true); + GC.SuppressFinalize(this); + } + + /// + /// Releases resources used by the and + /// if overridden in a derived class, optionally releases unmanaged resources. + /// + /// true to release both managed and unmanaged resources; + /// false to release only unmanaged resources. + + // LUCENENET specific - implemented proper dispose pattern + protected virtual void Dispose(bool disposing) + { + if (disposing && !disposed) { lock (searcherLock) { disposed = true; - if (searcher != null) - { - searcher.IndexReader.Dispose(); - } + searcher?.IndexReader?.Dispose(); searcher = null; } } @@ -687,12 +697,9 @@ private void SwapSearcher(Directory dir) if (disposed) { indexSearcher.IndexReader.Dispose(); - throw new ObjectDisposedException(this.GetType().FullName, "Spellchecker has been closed"); - } - if (searcher != null) - { - searcher.IndexReader.Dispose(); + throw new ObjectDisposedException(this.GetType().FullName, "Spellchecker has been disposed."); } + searcher?.IndexReader?.Dispose(); // set the spellindex in the sync block - ensure consistency. searcher = indexSearcher; this.spellIndex = dir; diff --git a/src/Lucene.Net/Codecs/Compressing/CompressingTermVectorsReader.cs b/src/Lucene.Net/Codecs/Compressing/CompressingTermVectorsReader.cs index 723de99ea7..e33061678f 100644 --- a/src/Lucene.Net/Codecs/Compressing/CompressingTermVectorsReader.cs +++ b/src/Lucene.Net/Codecs/Compressing/CompressingTermVectorsReader.cs @@ -31,7 +31,7 @@ namespace Lucene.Net.Codecs.Compressing /// /// @lucene.experimental /// - public sealed class CompressingTermVectorsReader : TermVectorsReader, IDisposable + public sealed class CompressingTermVectorsReader : TermVectorsReader // LUCENENET specific - removed IDisposable, it is already implemented in base class { private readonly FieldInfos fieldInfos; internal readonly CompressingStoredFieldsIndexReader indexReader; diff --git a/src/Lucene.Net/Codecs/DocValuesProducer.cs b/src/Lucene.Net/Codecs/DocValuesProducer.cs index 900d1b3b3f..f4da0f8553 100644 --- a/src/Lucene.Net/Codecs/DocValuesProducer.cs +++ b/src/Lucene.Net/Codecs/DocValuesProducer.cs @@ -95,7 +95,7 @@ protected internal DocValuesProducer() /// /// Disposes all resources used by this object. /// - public virtual void Dispose() + public void Dispose() { Dispose(true); GC.SuppressFinalize(this); diff --git a/src/Lucene.Net/Codecs/Lucene40/Lucene40StoredFieldsReader.cs b/src/Lucene.Net/Codecs/Lucene40/Lucene40StoredFieldsReader.cs index 6fed3e0193..ece87d04ef 100644 --- a/src/Lucene.Net/Codecs/Lucene40/Lucene40StoredFieldsReader.cs +++ b/src/Lucene.Net/Codecs/Lucene40/Lucene40StoredFieldsReader.cs @@ -41,7 +41,7 @@ namespace Lucene.Net.Codecs.Lucene40 /// @lucene.internal /// /// - public sealed class Lucene40StoredFieldsReader : StoredFieldsReader, IDisposable + public sealed class Lucene40StoredFieldsReader : StoredFieldsReader // LUCENENET specific - removed IDisposable, it is already implemented in base class #if FEATURE_CLONEABLE , System.ICloneable #endif diff --git a/src/Lucene.Net/Codecs/Lucene40/Lucene40TermVectorsReader.cs b/src/Lucene.Net/Codecs/Lucene40/Lucene40TermVectorsReader.cs index d2bd2e2629..daa5918905 100644 --- a/src/Lucene.Net/Codecs/Lucene40/Lucene40TermVectorsReader.cs +++ b/src/Lucene.Net/Codecs/Lucene40/Lucene40TermVectorsReader.cs @@ -46,7 +46,7 @@ namespace Lucene.Net.Codecs.Lucene40 /// It reads .tvd, .tvf, and .tvx files. /// /// - public class Lucene40TermVectorsReader : TermVectorsReader, IDisposable + public class Lucene40TermVectorsReader : TermVectorsReader // LUCENENET specific - removed IDisposable, it is already implemented in base class { internal const sbyte STORE_POSITIONS_WITH_TERMVECTOR = 0x1; diff --git a/src/Lucene.Net/Codecs/Lucene45/Lucene45DocValuesConsumer.cs b/src/Lucene.Net/Codecs/Lucene45/Lucene45DocValuesConsumer.cs index 7bd44515d2..9581c2cd99 100644 --- a/src/Lucene.Net/Codecs/Lucene45/Lucene45DocValuesConsumer.cs +++ b/src/Lucene.Net/Codecs/Lucene45/Lucene45DocValuesConsumer.cs @@ -39,7 +39,7 @@ namespace Lucene.Net.Codecs.Lucene45 /// /// Writer for - public class Lucene45DocValuesConsumer : DocValuesConsumer, IDisposable + public class Lucene45DocValuesConsumer : DocValuesConsumer // LUCENENET specific - removed IDisposable, it is already implemented in base class { internal static readonly int BLOCK_SIZE = 16384; internal static readonly int ADDRESS_INTERVAL = 16; diff --git a/src/Lucene.Net/Codecs/Lucene45/Lucene45DocValuesProducer.cs b/src/Lucene.Net/Codecs/Lucene45/Lucene45DocValuesProducer.cs index bbd6fd05d1..8975faebd9 100644 --- a/src/Lucene.Net/Codecs/Lucene45/Lucene45DocValuesProducer.cs +++ b/src/Lucene.Net/Codecs/Lucene45/Lucene45DocValuesProducer.cs @@ -51,7 +51,7 @@ namespace Lucene.Net.Codecs.Lucene45 /// /// Reader for . - public class Lucene45DocValuesProducer : DocValuesProducer, IDisposable + public class Lucene45DocValuesProducer : DocValuesProducer // LUCENENET specific - removed IDisposable, it is already implemented in base class { private readonly IDictionary numerics; private readonly IDictionary binaries; diff --git a/src/Lucene.Net/Search/ControlledRealTimeReopenThread.cs b/src/Lucene.Net/Search/ControlledRealTimeReopenThread.cs index 40dbd10509..040085a10e 100644 --- a/src/Lucene.Net/Search/ControlledRealTimeReopenThread.cs +++ b/src/Lucene.Net/Search/ControlledRealTimeReopenThread.cs @@ -113,25 +113,45 @@ private void RefreshDone() reopenCond.Reset(); } + /// + /// Releases all resources used by the . + /// public void Dispose() { - finish = true; - reopenCond.Set(); -//#if FEATURE_THREAD_INTERRUPT -// try -// { -//#endif + Dispose(true); + GC.SuppressFinalize(this); + } + + /// + /// Releases resources used by the and + /// if overridden in a derived class, optionally releases unmanaged resources. + /// + /// true to release both managed and unmanaged resources; + /// false to release only unmanaged resources. + + // LUCENENET specific - implemented proper dispose pattern + protected virtual void Dispose(bool disposing) + { + if (disposing) + { + finish = true; + reopenCond.Set(); + //#if FEATURE_THREAD_INTERRUPT + // try + // { + //#endif Join(); -//#if FEATURE_THREAD_INTERRUPT // LUCENENET NOTE: Senseless to catch and rethrow the same exception type -// } -// catch (ThreadInterruptedException ie) -// { -// throw new ThreadInterruptedException(ie.ToString(), ie); -// } -//#endif - // LUCENENET specific: dispose reset event - reopenCond.Dispose(); - available.Dispose(); + //#if FEATURE_THREAD_INTERRUPT // LUCENENET NOTE: Senseless to catch and rethrow the same exception type + // } + // catch (ThreadInterruptedException ie) + // { + // throw new ThreadInterruptedException(ie.ToString(), ie); + // } + //#endif + // LUCENENET specific: dispose reset event + reopenCond.Dispose(); + available.Dispose(); + } } /// diff --git a/src/Lucene.Net/Search/LiveFieldValues.cs b/src/Lucene.Net/Search/LiveFieldValues.cs index d9f13a3a55..b569bdc9e0 100644 --- a/src/Lucene.Net/Search/LiveFieldValues.cs +++ b/src/Lucene.Net/Search/LiveFieldValues.cs @@ -33,7 +33,6 @@ namespace Lucene.Net.Search /// the same time by two threads, because in this case you /// cannot in general know which thread "won". /// - public abstract class LiveFieldValues : ReferenceManager.IRefreshListener, IDisposable where S : class { @@ -49,9 +48,29 @@ public LiveFieldValues(ReferenceManager mgr, T missingValue) mgr.AddListener(this); } + /// + /// Releases all resources used by the . + /// public void Dispose() { - mgr.RemoveListener(this); + Dispose(true); + GC.SuppressFinalize(this); + } + + /// + /// Releases resources used by the and + /// if overridden in a derived class, optionally releases unmanaged resources. + /// + /// true to release both managed and unmanaged resources; + /// false to release only unmanaged resources. + + // LUCENENET specific - implemented proper dispose pattern + protected virtual void Dispose(bool disposing) + { + if (disposing) + { + mgr.RemoveListener(this); + } } public virtual void BeforeRefresh() diff --git a/src/Lucene.Net/Search/ReferenceManager.cs b/src/Lucene.Net/Search/ReferenceManager.cs index a6374e9dfe..3dcfe3e119 100644 --- a/src/Lucene.Net/Search/ReferenceManager.cs +++ b/src/Lucene.Net/Search/ReferenceManager.cs @@ -154,11 +154,8 @@ the reference. */ /// If the underlying reader of the current reference could not be disposed public void Dispose() { - lock (this) - { - Dispose(true); - GC.SuppressFinalize(this); - } + Dispose(true); + GC.SuppressFinalize(this); } /// @@ -167,17 +164,25 @@ public void Dispose() protected abstract int GetRefCount(G reference); /// - /// Called after , so subclass can free any resources. + /// Called after , so subclass can free any resources. + /// + /// When overriding, be sure to include a call to base.Dispose(disposing) in your implementation. /// if the after dispose operation in a sub-class throws an /// protected virtual void Dispose(bool disposing) { - if (disposing && current != null) + if (disposing) { - // make sure we can call this more than once - // closeable javadoc says: - // if this is already closed then invoking this method has no effect. - SwapReference(null); + lock (this) + { + if (current != null) + { + // make sure we can call this more than once + // closeable javadoc says: + // if this is already closed then invoking this method has no effect. + SwapReference(null); + } + } } } diff --git a/src/Lucene.Net/Search/SearcherLifetimeManager.cs b/src/Lucene.Net/Search/SearcherLifetimeManager.cs index 84aaa1727e..7385c68679 100644 --- a/src/Lucene.Net/Search/SearcherLifetimeManager.cs +++ b/src/Lucene.Net/Search/SearcherLifetimeManager.cs @@ -312,27 +312,43 @@ public virtual void Prune(IPruner pruner) /// otherwise it's possible not all searcher references /// will be freed. /// - public virtual void Dispose() + public void Dispose() { - lock (this) - { - _closed = true; - IList toClose = new List(_searchers.Values.Select(item => item.Value)); + Dispose(true); + GC.SuppressFinalize(this); + } + + /// + /// Releases resources used by the and + /// if overridden in a derived class, optionally releases unmanaged resources. + /// + /// true to release both managed and unmanaged resources; + /// false to release only unmanaged resources. - // Remove up front in case exc below, so we don't - // over-decRef on double-close: - foreach (var tracker in toClose) + // LUCENENET specific - implemented proper dispose pattern + protected virtual void Dispose(bool disposing) + { + if (disposing) + { + lock (this) { - Lazy _; - _searchers.TryRemove(tracker.Version, out _); - } + _closed = true; + IList toClose = new List(_searchers.Values.Select(item => item.Value)); + + // Remove up front in case exc below, so we don't + // over-decRef on double-close: + foreach (var tracker in toClose) + { + _searchers.TryRemove(tracker.Version, out Lazy _); + } - IOUtils.Dispose(toClose); + IOUtils.Dispose(toClose); - // Make some effort to catch mis-use: - if (_searchers.Count != 0) - { - throw new InvalidOperationException("another thread called record while this SearcherLifetimeManager instance was being closed; not all searchers were closed"); + // Make some effort to catch mis-use: + if (_searchers.Count != 0) + { + throw new InvalidOperationException("another thread called record while this SearcherLifetimeManager instance was being disposed; not all searchers were disposed"); + } } } } diff --git a/src/Lucene.Net/Store/OutputStreamDataOutput.cs b/src/Lucene.Net/Store/OutputStreamDataOutput.cs index 7e448f0335..f6629ae1be 100644 --- a/src/Lucene.Net/Store/OutputStreamDataOutput.cs +++ b/src/Lucene.Net/Store/OutputStreamDataOutput.cs @@ -42,9 +42,29 @@ public override void WriteBytes(byte[] b, int offset, int length) _writer.Write(b, offset, length); } - public virtual void Dispose() + /// + /// Releases all resources used by the . + /// + public void Dispose() { - _writer.Dispose(); + Dispose(true); + GC.SuppressFinalize(this); + } + + /// + /// Releases resources used by the and + /// if overridden in a derived class, optionally releases unmanaged resources. + /// + /// true to release both managed and unmanaged resources; + /// false to release only unmanaged resources. + + // LUCENENET specific - implemented proper dispose pattern + protected virtual void Dispose(bool disposing) + { + if (disposing) + { + _writer.Dispose(); + } } } } \ No newline at end of file