From 73534c5a3d04efc3955c29f2560301eb1f79a7be Mon Sep 17 00:00:00 2001 From: Shad Storhaug Date: Fri, 12 Mar 2021 17:20:14 +0700 Subject: [PATCH] Lucene.Net.Facet.Taxonomy.CachedOrdinalsReader: Fixed synchronization issues with adding new items to the cache and reading RamBytesUsed() method (see #417) --- .../Taxonomy/CachedOrdinalsReader.cs | 56 ++++++------------- 1 file changed, 16 insertions(+), 40 deletions(-) diff --git a/src/Lucene.Net.Facet/Taxonomy/CachedOrdinalsReader.cs b/src/Lucene.Net.Facet/Taxonomy/CachedOrdinalsReader.cs index 7ff6132919..fa8760738b 100644 --- a/src/Lucene.Net.Facet/Taxonomy/CachedOrdinalsReader.cs +++ b/src/Lucene.Net.Facet/Taxonomy/CachedOrdinalsReader.cs @@ -66,11 +66,10 @@ public class CachedOrdinalsReader : OrdinalsReader, IAccountable #if FEATURE_CONDITIONALWEAKTABLE_ENUMERATOR private readonly ConditionalWeakTable ordsCache = new ConditionalWeakTable(); - private readonly object ordsCacheLock = new object(); #else private readonly WeakDictionary ordsCache = new WeakDictionary(); - private readonly ReaderWriterLockSlim syncLock = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion); #endif + private readonly object syncLock = new object(); /// /// Sole constructor. @@ -81,35 +80,23 @@ public CachedOrdinalsReader(OrdinalsReader source) private CachedOrds GetCachedOrds(AtomicReaderContext context) { - object cacheKey = context.Reader.CoreCacheKey; -#if FEATURE_CONDITIONALWEAKTABLE_ENUMERATOR - return ordsCache.GetValue(cacheKey, (cacheKey) => new CachedOrds(source.GetReader(context), context.Reader.MaxDoc)); -#else - CachedOrds ords; - syncLock.EnterReadLock(); - try + // LUCENENET NOTE: Since ConditionalWeakTable doesn't synchronize on enumeration in the RamBytesUsed() method, + // the lock is still required here despite it being a threadsafe collection. + lock (syncLock) { - if (ordsCache.TryGetValue(cacheKey, out ords)) - return ords; - } - finally - { - syncLock.ExitReadLock(); - } + object cacheKey = context.Reader.CoreCacheKey; + if (!ordsCache.TryGetValue(cacheKey, out CachedOrds ords)) + { + ords = new CachedOrds(source.GetReader(context), context.Reader.MaxDoc); - ords = new CachedOrds(source.GetReader(context), context.Reader.MaxDoc); - syncLock.EnterWriteLock(); - try - { - ordsCache[cacheKey] = ords; - } - finally - { - syncLock.ExitWriteLock(); + // LUCENENET specific: Since this is the only thread that can modify ordsCache + // and we just checked that the value doesn't exist above, we can simplify this to Add() + // which also means we don't need conditional compilation because ConditionalWeakTable + // doesn't support this[index]. + ordsCache.Add(cacheKey, ords); + } + return ords; } - - return ords; -#endif } public override string IndexFieldName => source.IndexFieldName; @@ -211,12 +198,7 @@ public long RamBytesUsed() public virtual long RamBytesUsed() { -#if FEATURE_CONDITIONALWEAKTABLE_ENUMERATOR - lock (ordsCacheLock) -#else - syncLock.EnterReadLock(); - try -#endif + lock (syncLock) { long bytes = 0; foreach (var pair in ordsCache) @@ -226,12 +208,6 @@ public virtual long RamBytesUsed() return bytes; } -#if !FEATURE_CONDITIONALWEAKTABLE_ENUMERATOR - finally - { - syncLock.ExitReadLock(); - } -#endif } } } \ No newline at end of file