Improves thread safety while concurrent gets and deletes happen

FluidCaching.sln.DotSettings

@@ -26,6 +26,7 @@
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpKeepExistingMigration/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpPlaceEmbeddedOnSameLineMigration/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpRenamePlacementToArrangementMigration/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpUseContinuousIndentInsideBracesMigration/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EAddAccessorOwnerDeclarationBracesMigration/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002ECSharpPlaceAttributeOnSameLineMigration/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateBlankLinesAroundFieldToBlankLinesAroundProperty/@EntryIndexedValue">True</s:Boolean>

Src/FluidCaching/FluidCache.cs

@@ -34,7 +34,8 @@ class FluidCache<T> where T : class
     {
         private readonly Dictionary<string, IIndexManagement<T>> indexList = new Dictionary<string, IIndexManagement<T>>();
         private readonly LifespanManager<T> lifeSpan;
-
+        private readonly object syncObject = new object();
+
         /// <summary>Constructor</summary>
         /// <param name="capacity">the normal item limit for cache (Count may exeed capacity due to minAge)</param>
         /// <param name="minAge">the minimium time after an access before an item becomes eligible for removal, during this time
@@ -66,11 +67,19 @@ public IIndex<TKey, T> GetIndex<TKey>(string indexName)
             return indexList.TryGetValue(indexName, out index) ? index as IIndex<TKey, T> : null;
         }
 
-        /// <summary>Retrieve a object by index name / key</summary>
-        public Task<T> Get<TKey>(string indexName, TKey key, ItemCreator<TKey, T> item = null)
+        /// <summary>
+        /// Gets an object associated with <paramref name="key"/> from the index identified by <paramref name="indexName"/>
+        /// or tries to create a new one using the
+        /// (optional) factory method provided by <paramref name="createItem"/>
+        /// </summary>
+        /// <returns>
+        /// Returns the object associated with the key or <c>null</c> if no such object exists and
+        /// the <paramref name="createItem"/> was <c>null</c> or returned a <c>null</c>.
+        /// </returns>
+        public Task<T> Get<TKey>(string indexName, TKey key, ItemCreator<TKey, T> createItem = null)
         {
             IIndex<TKey, T> index = GetIndex<TKey>(indexName);
-            return index?.GetItem(key, item);
+            return index?.GetItem(key, createItem);
         }
 
         /// <summary>Adds a new index to the cache</summary>
@@ -96,20 +105,20 @@ public IIndex<TKey, T> AddIndex<TKey>(
         /// </summary>
         public void Add(T item)
         {
-            AddAsNode(item);
+            TryAddAsNode(item);
         }
 
         /// <summary>
         /// AddAsNode an item to the cache
         /// </summary>
-        internal INode<T> AddAsNode(T item)
+        internal Node<T> TryAddAsNode(T item)
         {
             if (item == null)
             {
                 return null;
             }
 
-            INode<T> node = FindExistingNode(item);
+            Node<T> node = FindExistingNode(item);
 
             // dupl is used to prevent total count from growing when item is already in indexes (only new Nodes)
             bool isDuplicate = (node != null) && (node.Value == item);
@@ -125,7 +134,7 @@ internal INode<T> AddAsNode(T item)
                     }
                 }
 
-                lock (this)
+                lock (syncObject)
                 {
                     if (!isDuplicate)
                     {
@@ -143,9 +152,9 @@ internal INode<T> AddAsNode(T item)
             return node;
         }
 
-        private INode<T> FindExistingNode(T item)
+        private Node<T> FindExistingNode(T item)
         {
-            INode<T> node = null;
+            Node<T> node = null;
             foreach (KeyValuePair<string, IIndexManagement<T>> keyValue in indexList)
             {
                 if ((node = keyValue.Value.FindItem(item)) != null)

Src/FluidCaching/IIndex.cs

@@ -13,13 +13,13 @@ namespace FluidCaching
         interface IIndex<TKey, T> where T : class
     {
         /// <summary>
-        /// Getter for index
+        /// Gets an object from the index based on the provided <paramref name="key"/> or tries to create a new one using the
+        /// (optional) factory method provided by <paramref name="createItem"/>
         /// </summary>
-        /// <param name="key">key to find (or load if needed)</param>
-        /// <param name="createItem">
-        /// An optional delegate that is used to create the actual object if it doesn't exist in the cache.
-        /// </param>
-        /// <returns>the object value associated with the cache</returns>
+        /// <returns>
+        /// Returns the object associated with the key or <c>null</c> if no such object exists and
+        /// the <paramref name="createItem"/> was <c>null</c> or returned a <c>null</c>.
+        /// </returns>
         Task<T> GetItem(TKey key, ItemCreator<TKey, T> createItem = null);
 
         /// <summary>Delete object that matches key from cache</summary>

Src/FluidCaching/IIndexManagement.cs

@@ -6,8 +6,8 @@ namespace FluidCaching
     internal interface IIndexManagement<T> where T : class
     {
         void ClearIndex();
-        bool AddItem(INode<T> item);
-        INode<T> FindItem(T item);
+        bool AddItem(Node<T> item);
+        Node<T> FindItem(T item);
         int RebuildIndex();
     }
 }

Src/FluidCaching/Index.cs

@@ -13,11 +13,12 @@ internal class Index<TKey, T> : IIndex<TKey, T>, IIndexManagement<T> where T : c
     {
         private readonly FluidCache<T> owner;
         private readonly LifespanManager<T> lifespanManager;
-        private Dictionary<TKey, WeakReference<INode<T>>> index;
+        private Dictionary<TKey, WeakReference<Node<T>>> index;
         private readonly GetKey<T, TKey> _getKey;
         private readonly ItemCreator<TKey, T> loadItem;
         private readonly IEqualityComparer<TKey> keyEqualityComparer;
-
+        private readonly object syncObject = new object();
+
         /// <summary>constructor</summary>
         /// <param name="owner">parent of index</param>
         /// <param name="lifespanManager"></param>
@@ -41,19 +42,40 @@ public Index(
             RebuildIndex();
         }
 
-        /// <summary>Getter for index</summary>
-        /// <param name="key">key to find (or load if needed)</param>
-        /// <param name="createItem">
-        /// An optional factory method for creating the item if it does not exist in the cache.
-        /// </param>
-        /// <returns>the object value associated with key, or null if not found or could not be loaded</returns>
         public async Task<T> GetItem(TKey key, ItemCreator<TKey, T> createItem = null)
         {
-            INode<T> node = FindExistingNodeByKey(key);
-            node?.Touch();
+            T value = null;
+
+            lock (syncObject)
+            {
+                Node<T> node = FindExistingNodeByKey(key);
+                if (node != null)
+                {
+                    value = node.Value;
+
+                    node.Touch();
+                }
+            }
+
+            if (value == null)
+            {
+                value = await TryCreate(key, createItem);
+                if (value != null)
+                {
+                    Node<T> newOrExistingNode = owner.TryAddAsNode(value);
+                    value = newOrExistingNode.Value;
+
+                    lifespanManager.CheckValidity();
+                }
+            }
 
+            return value;
+        }
+
+        private async Task<T> TryCreate(TKey key, ItemCreator<TKey, T> createItem = null)
+        {
             ItemCreator<TKey, T> creator = createItem ?? loadItem;
-            if ((node?.Value == null) && (creator != null))
+            if (creator != null)
             {
                 Task<T> task = creator(key);
                 if (task == null)
@@ -62,31 +84,20 @@ public async Task<T> GetItem(TKey key, ItemCreator<TKey, T> createItem = null)
                         "Expected a non-null Task. Did you intend to return a null-returning Task instead?");
                 }
 
-                T value = await task;
-
-                lock (this)
-                {
-                    node = FindExistingNodeByKey(key);
-                    if (node?.Value == null)
-                    {
-                        node = owner.AddAsNode(value);
-                    }
-
-                    lifespanManager.CheckValidity();
-                }
+                return await task;
             }
 
-            return node?.Value;
+            return null;
         }
 
         /// <summary>Delete object that matches key from cache</summary>
         /// <param name="key"></param>
         public void Remove(TKey key)
         {
-            INode<T> node = FindExistingNodeByKey(key);
+            Node<T> node = FindExistingNodeByKey(key);
             if (node != null)
             {
-                lock (this)
+                lock (syncObject)
                 {
                     node = FindExistingNodeByKey(key);
                     if (node != null)
@@ -100,15 +111,15 @@ public void Remove(TKey key)
         }
 
         /// <summary>try to find this item in the index and return Node</summary>
-        public INode<T> FindItem(T item)
+        public Node<T> FindItem(T item)
         {
             return FindExistingNodeByKey(_getKey(item));
         }
 
-        private INode<T> FindExistingNodeByKey(TKey key)
+        private Node<T> FindExistingNodeByKey(TKey key)
         {
-            WeakReference<INode<T>> reference;
-            INode<T> node;
+            WeakReference<Node<T>> reference;
+            Node<T> node;
             if (index.TryGetValue(key, out reference) && reference.TryGetTarget(out node))
             {
                 lifespanManager.Stats.RegisterHit();
@@ -121,7 +132,7 @@ private INode<T> FindExistingNodeByKey(TKey key)
         /// <summary>Remove all items from index</summary>
         public void ClearIndex()
         {
-            lock (this)
+            lock (syncObject)
             {
                 index.Clear();
             }
@@ -132,16 +143,16 @@ public void ClearIndex()
         /// <returns>
         /// Returns <c>true</c> if the item could be added to the index, or <c>false</c> otherwise.
         /// </returns>
-        public bool AddItem(INode<T> item)
+        public bool AddItem(Node<T> item)
         {
-            lock (this)
+            lock (syncObject)
             {
                 TKey key = _getKey(item.Value);
 
-                INode<T> node;
+                Node<T> node;
                 if (!index.ContainsKey(key) || !index[key].TryGetTarget(out node) || node.Value == null)
                 {
-                    index[key] = new WeakReference<INode<T>>(item, trackResurrection: false);
+                    index[key] = new WeakReference<Node<T>>(item, trackResurrection: false);
                     return true;
                 }
 
@@ -152,13 +163,13 @@ public bool AddItem(INode<T> item)
         /// <summary>removes all items from index and reloads each item (this gets rid of dead nodes)</summary>
         public int RebuildIndex()
         {
-            lock (this)
+            lock (syncObject)
             {
                 // Create a new ConcurrentDictionary, this way there is no need for locking the index itself
                 var keyValues = lifespanManager
-                    .ToDictionary(item => _getKey(item.Value), item => new WeakReference<INode<T>>(item));
+                    .ToDictionary(item => _getKey(item.Value), item => new WeakReference<Node<T>>(item));
 
-                index = new Dictionary<TKey, WeakReference<INode<T>>>(keyValues, keyEqualityComparer);
+                index = new Dictionary<TKey, WeakReference<Node<T>>>(keyValues, keyEqualityComparer);
                 return index.Count;
             }
         }

Src/FluidCaching/LifespanManager.cs

@@ -5,7 +5,7 @@
 
 namespace FluidCaching
 {
-    internal class LifespanManager<T> : IEnumerable<INode<T>> where T : class
+    internal class LifespanManager<T> : IEnumerable<Node<T>> where T : class
     {
         /// <summary>
         /// The number of bags which should be enough to store the requested capacity of items. The heuristic is that each
@@ -40,6 +40,7 @@ internal class LifespanManager<T> : IEnumerable<INode<T>> where T : class
         internal int itemsInCurrentBag;
         private int currentBagIndex;
         private int oldestBagIndex;
+        private readonly object syncObject = new object();
 
         public LifespanManager(FluidCache<T> owner, int capacity, TimeSpan minAge, TimeSpan maxAge, GetNow getNow)
         {
@@ -74,7 +75,7 @@ public void CheckValidity()
         {
             // NOTE: Monitor.Enter(this) / Monitor.Exit(this) is the same as lock(this)... We are using Monitor.TryEnter() because it
             // does not wait for a lock, if lock is currently held then skip and let next Touch perform cleanup.
-            if (RequiresCleanup && Monitor.TryEnter(this))
+            if (RequiresCleanup && Monitor.TryEnter(syncObject))
             {
                 try
                 {
@@ -93,7 +94,7 @@ public void CheckValidity()
                 }
                 finally
                 {
-                    Monitor.Exit(this);
+                    Monitor.Exit(syncObject);
                 }
             }
         }
@@ -111,7 +112,7 @@ public void CheckValidity()
         /// </remarks>
         private void CleanUp(DateTime now)
         {
-            lock (this)
+            lock (syncObject)
             {
                 int itemsAboveCapacity = Stats.Current - Stats.Capacity;
                 AgeBag<T> bag = bags[OldestBagIndex];
@@ -209,7 +210,7 @@ public int CurrentBagIndex
         /// <summary>Remove all items from LifespanMgr and reset</summary>
         public void Clear()
         {
-            lock (this)
+            lock (syncObject)
             {
                 bags.Empty();
 
@@ -223,7 +224,7 @@ public void Clear()
         /// <summary>ready a new current AgeBag for use and close the previous one</summary>
         private void OpenBag(int bagNumber)
         {
-            lock (this)
+            lock (syncObject)
             {
                 DateTime now = getNow();
 
@@ -255,7 +256,7 @@ IEnumerator IEnumerable.GetEnumerator()
         }
 
         /// <summary>Create item enumerator</summary>
-        public IEnumerator<INode<T>> GetEnumerator()
+        public IEnumerator<Node<T>> GetEnumerator()
         {
             for (int bagNumber = CurrentBagIndex; bagNumber >= OldestBagIndex; --bagNumber)
             {
@@ -273,7 +274,7 @@ public IEnumerator<INode<T>> GetEnumerator()
 
         public Node<T> AddToHead(Node<T> node)
         {
-            lock (this)
+            lock (syncObject)
             {
                 Node<T> next = CurrentBag.First;
                 CurrentBag.First = node;