Atomic caches are in an inconsistent state when factory throws

BitFaster.Caching.UnitTests/Atomic/AtomicFactoryAsyncCacheTests.cs

@@ -203,6 +203,43 @@ public void WhenItemsAddedEnumerateContainsKvps()
             enumerable.Should().BeEquivalentTo(new[] { new KeyValuePair<int, int>(1, 1), new KeyValuePair<int, int>(2, 2) });
         }
 
+        [Fact]
+        public async Task WhenFactoryThrowsEmptyValueIsNotCounted()
+        {
+            try
+            {
+                await cache.GetOrAddAsync(1, k => throw new ArithmeticException());
+            }
+            catch { }
+
+            cache.Count.Should().Be(0);
+        }
+
+        [Fact]
+        public async Task WhenFactoryThrowsEmptyValueIsNotEnumerable()
+        {
+            try
+            {
+                await cache.GetOrAddAsync(1, k => throw new ArithmeticException());
+            }
+            catch { }
+
+            // IEnumerable.Count() instead of Count property
+            cache.Count().Should().Be(0);
+        }
+
+        [Fact]
+        public async Task WhenFactoryThrowsEmptyKeyIsNotEnumerable()
+        {
+            try
+            {
+                await cache.GetOrAddAsync(1, k => throw new ArithmeticException());
+            }
+            catch { }
+
+            cache.Keys.Count().Should().Be(0);
+        }
+
         private void OnItemRemoved(object sender, ItemRemovedEventArgs<int, int> e)
         {
             this.removedItems.Add(e);

BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheTests.cs

@@ -203,6 +203,43 @@ public void WhenItemsAddedEnumerateContainsKvps()
             enumerable.Should().BeEquivalentTo(new[] { new KeyValuePair<int, int>(1, 1), new KeyValuePair<int, int>(2, 2) });
         }
 
+        [Fact]
+        public void WhenFactoryThrowsEmptyValueIsNotCounted()
+        {
+            try
+            {
+                cache.GetOrAdd(1, _ => throw new Exception());
+            }
+            catch { }
+
+            cache.Count.Should().Be(0);
+        }
+
+        [Fact]
+        public void WhenFactoryThrowsEmptyValueIsNotEnumerable()
+        {
+            try
+            {
+                cache.GetOrAdd(1, k => throw new Exception());
+            }
+            catch { }
+
+            // IEnumerable.Count() instead of Count property
+            cache.Count().Should().Be(0);
+        }
+
+        [Fact]
+        public void WhenFactoryThrowsEmptyKeyIsNotEnumerable()
+        {
+            try
+            {
+                cache.GetOrAdd(1, k => throw new Exception());
+            }
+            catch { }
+
+            cache.Keys.Count().Should().Be(0);
+        }
+
         private void OnItemRemoved(object sender, ItemRemovedEventArgs<int, int> e)
         {
             this.removedItems.Add(e);

BitFaster.Caching.UnitTests/Atomic/AtomicFactoryScopedCacheTests.cs

@@ -83,5 +83,42 @@ public void WhenEntryIsUpdatedOldEntryIsDisposed()
             this.cache.TryUpdate(1, new Disposable()).Should().BeTrue();
             disposable2.IsDisposed.Should().BeTrue();
         }
+
+        [Fact]
+        public void WhenFactoryThrowsEmptyValueIsNotCounted()
+        {
+            try
+            {
+                cache.ScopedGetOrAdd(1, _ => throw new Exception());
+            }
+            catch { }
+
+            cache.Count.Should().Be(0);
+        }
+
+        [Fact]
+        public void WhenFactoryThrowsEmptyValueIsNotEnumerable()
+        {
+            try
+            {
+                cache.ScopedGetOrAdd(1, k => throw new Exception());
+            }
+            catch { }
+
+            // IEnumerable.Count() instead of Count property
+            cache.Count().Should().Be(0);
+        }
+
+        [Fact]
+        public void WhenFactoryThrowsEmptyKeyIsNotEnumerable()
+        {
+            try
+            {
+                cache.ScopedGetOrAdd(1, k => throw new Exception());
+            }
+            catch { }
+
+            cache.Keys.Count().Should().Be(0);
+        }
     }
 }

BitFaster.Caching.UnitTests/ScopedAsyncCacheTestBase.cs

@@ -228,6 +228,43 @@ public void WhenItemsAddedEnumerateContainsKvps()
             list.Should().BeEquivalentTo(new[] { new KeyValuePair<int, Disposable>(1, d1), new KeyValuePair<int, Disposable>(2, d2) });
         }
 
+        [Fact]
+        public async Task WhenFactoryThrowsEmptyValueIsNotCounted()
+        {
+            try
+            {
+                await cache.ScopedGetOrAddAsync(1, k => throw new ArithmeticException());
+            }
+            catch { }
+
+            cache.Count.Should().Be(0);
+        }
+
+        [Fact]
+        public async Task WhenFactoryThrowsEmptyValueIsNotEnumerable()
+        {
+            try
+            {
+                await cache.ScopedGetOrAddAsync(1, k => throw new ArithmeticException());
+            }
+            catch { }
+
+            // IEnumerable.Count() instead of Count property
+            cache.Count().Should().Be(0);
+        }
+
+        [Fact]
+        public async Task WhenFactoryThrowsEmptyKeyIsNotEnumerable()
+        {
+            try
+            {
+                await cache.ScopedGetOrAddAsync(1, k => throw new ArithmeticException());
+            }
+            catch { }
+
+            cache.Keys.Count().Should().Be(0);
+        }
+
         protected void OnItemRemoved(object sender, ItemRemovedEventArgs<int, Scoped<Disposable>> e)
         {
             this.removedItems.Add(e);

BitFaster.Caching/Atomic/AtomicEx.cs

@@ -0,0 +1,39 @@
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.Linq;
+using System.Text;
+
+namespace BitFaster.Caching.Atomic
+{
+    internal static class AtomicEx
+    {
+        internal static int EnumerateCount(IEnumerator enumerator)
+        {
+            int count = 0;
+            while (enumerator.MoveNext())
+            {
+                count++;
+            }
+            return count;
+        }
+
+        internal static ICollection<K> FilterKeys<K, V>(IEnumerable<KeyValuePair<K, V>> kvps, Func<V, bool> filter)
+        {
+            // Here we will double enumerate the kvps list. Alternative is to lazy init the size which will keep resizing
+            // the List, and spam allocs if the list is long.
+            List<K> keys = new List<K>(kvps.Count());
+
+            foreach (var kvp in kvps)
+            {
+                if (filter(kvp.Value))
+                {
+                    keys.Add(kvp.Key);
+                }
+            }
+
+            return new ReadOnlyCollection<K>(keys);
+        }
+    }
+}

BitFaster.Caching/Atomic/AtomicFactoryAsyncCache.cs

@@ -43,7 +43,7 @@ public AtomicFactoryAsyncCache(ICache<K, AsyncAtomicFactory<K, V>> cache)
         }
 
         ///<inheritdoc/>
-        public int Count => cache.Count;
+        public int Count => AtomicEx.EnumerateCount(this.GetEnumerator());
 
         ///<inheritdoc/>
         public Optional<ICacheMetrics> Metrics => cache.Metrics;
@@ -52,7 +52,7 @@ public AtomicFactoryAsyncCache(ICache<K, AsyncAtomicFactory<K, V>> cache)
         public Optional<ICacheEvents<K, V>> Events => this.events;
 
         ///<inheritdoc/>
-        public ICollection<K> Keys => this.cache.Keys;
+        public ICollection<K> Keys => AtomicEx.FilterKeys<K, AsyncAtomicFactory<K, V>>(this.cache, v => v.IsValueCreated);
 
         ///<inheritdoc/>
         public CachePolicy Policy => this.cache.Policy;
@@ -109,7 +109,10 @@ public IEnumerator<KeyValuePair<K, V>> GetEnumerator()
         {
             foreach (var kvp in this.cache)
             {
-                yield return new KeyValuePair<K, V>(kvp.Key, kvp.Value.ValueIfCreated);
+                if (kvp.Value.IsValueCreated)
+                { 
+                    yield return new KeyValuePair<K, V>(kvp.Key, kvp.Value.ValueIfCreated); 
+                }
             }
         }
 

BitFaster.Caching/Atomic/AtomicFactoryCache.cs

@@ -41,7 +41,7 @@ public AtomicFactoryCache(ICache<K, AtomicFactory<K, V>> cache)
         }
 
         ///<inheritdoc/>
-        public int Count => this.cache.Count;
+        public int Count => AtomicEx.EnumerateCount(this.GetEnumerator());
 
         ///<inheritdoc/>
         public Optional<ICacheMetrics> Metrics => this.cache.Metrics;
@@ -50,7 +50,7 @@ public AtomicFactoryCache(ICache<K, AtomicFactory<K, V>> cache)
         public Optional<ICacheEvents<K, V>> Events => this.events;
 
         ///<inheritdoc/>
-        public ICollection<K> Keys => this.cache.Keys;
+        public ICollection<K> Keys => AtomicEx.FilterKeys<K, AtomicFactory<K, V>>(this.cache, v => v.IsValueCreated);
 
         ///<inheritdoc/>
         public CachePolicy Policy => this.cache.Policy;
@@ -107,7 +107,10 @@ public IEnumerator<KeyValuePair<K, V>> GetEnumerator()
         {
             foreach (var kvp in this.cache)
             {
-                yield return new KeyValuePair<K, V>(kvp.Key, kvp.Value.ValueIfCreated);
+                if (kvp.Value.IsValueCreated)
+                {
+                    yield return new KeyValuePair<K, V>(kvp.Key, kvp.Value.ValueIfCreated);
+                }
             }
         }
 

BitFaster.Caching/Atomic/AtomicFactoryScopedAsyncCache.cs

@@ -42,7 +42,7 @@ public AtomicFactoryScopedAsyncCache(ICache<K, ScopedAsyncAtomicFactory<K, V>> c
         }
 
         ///<inheritdoc/>
-        public int Count => this.cache.Count;
+        public int Count => AtomicEx.EnumerateCount(this.GetEnumerator());
 
         ///<inheritdoc/>
         public Optional<ICacheMetrics> Metrics => this.cache.Metrics;
@@ -54,7 +54,7 @@ public AtomicFactoryScopedAsyncCache(ICache<K, ScopedAsyncAtomicFactory<K, V>> c
         public CachePolicy Policy => this.cache.Policy;
 
         ///<inheritdoc/>
-        public ICollection<K> Keys => this.cache.Keys;
+        public ICollection<K> Keys => AtomicEx.FilterKeys<K, ScopedAsyncAtomicFactory<K, V>>(this.cache, v => v.IsScopeCreated);
 
         ///<inheritdoc/>
         public void AddOrUpdate(K key, V value)
@@ -125,7 +125,10 @@ public IEnumerator<KeyValuePair<K, Scoped<V>>> GetEnumerator()
         {
             foreach (var kvp in this.cache)
             {
-                yield return new KeyValuePair<K, Scoped<V>>(kvp.Key, kvp.Value.ScopeIfCreated);
+                if (kvp.Value.IsScopeCreated)
+                { 
+                    yield return new KeyValuePair<K, Scoped<V>>(kvp.Key, kvp.Value.ScopeIfCreated); 
+                }
             }
         }
 

BitFaster.Caching/Atomic/AtomicFactoryScopedCache.cs

@@ -42,7 +42,7 @@ public AtomicFactoryScopedCache(ICache<K, ScopedAtomicFactory<K, V>> cache)
         }
 
         ///<inheritdoc/>
-        public int Count => this.cache.Count;
+        public int Count => AtomicEx.EnumerateCount(this.GetEnumerator());
 
         ///<inheritdoc/>
         public Optional<ICacheMetrics> Metrics => this.cache.Metrics;
@@ -54,7 +54,7 @@ public AtomicFactoryScopedCache(ICache<K, ScopedAtomicFactory<K, V>> cache)
         public CachePolicy Policy => this.cache.Policy;
 
         ///<inheritdoc/>
-        public ICollection<K> Keys => this.cache.Keys;
+        public ICollection<K> Keys => AtomicEx.FilterKeys<K, ScopedAtomicFactory<K, V>>(this.cache, v => v.IsScopeCreated);
 
         ///<inheritdoc/>
         public void AddOrUpdate(K key, V value)
@@ -123,7 +123,10 @@ public IEnumerator<KeyValuePair<K, Scoped<V>>> GetEnumerator()
         {
             foreach (var kvp in this.cache)
             {
-                yield return new KeyValuePair<K, Scoped<V>>(kvp.Key, kvp.Value.ScopeIfCreated);
+                if (kvp.Value.IsScopeCreated)
+                { 
+                    yield return new KeyValuePair<K, Scoped<V>>(kvp.Key, kvp.Value.ScopeIfCreated); 
+                }
             }
         }
 

BitFaster.Caching/Atomic/ScopedAsyncAtomicFactory.cs

@@ -11,7 +11,7 @@ namespace BitFaster.Caching.Atomic
     /// </summary>
     /// <typeparam name="K">The type of the key.</typeparam>
     /// <typeparam name="V">The type of the value.</typeparam>
-    [DebuggerDisplay("IsValueCreated={initializer == null}, Value={ScopeIfCreated}")]
+    [DebuggerDisplay("IsScopeCreated={initializer == null}, Value={ScopeIfCreated}")]
     public sealed class ScopedAsyncAtomicFactory<K, V> : IScoped<V>, IDisposable where V : IDisposable
     {
         private Scoped<V> scope;
@@ -35,6 +35,11 @@ public ScopedAsyncAtomicFactory(V value)
             scope = new Scoped<V>(value);
         }
 
+        /// <summary>
+        /// Gets a value indicating whether the scope has been initialized.
+        /// </summary>
+        public bool IsScopeCreated => initializer == null;
+
         /// <summary>
         /// Gets the scope if it has been initialized, else default.
         /// </summary>

BitFaster.Caching/Atomic/ScopedAtomicFactory.cs

@@ -26,7 +26,7 @@ namespace BitFaster.Caching.Atomic
     ///    </item>
     ///</list>
     /// </remarks>
-    [DebuggerDisplay("IsValueCreated={initializer == null}, Value={ScopeIfCreated}")]
+    [DebuggerDisplay("IsScopeCreated={initializer == null}, Value={ScopeIfCreated}")]
     public sealed class ScopedAtomicFactory<K, V> : IScoped<V>, IDisposable where V : IDisposable
     {
         private Scoped<V> scope;
@@ -50,6 +50,11 @@ public ScopedAtomicFactory(V value)
             scope = new Scoped<V>(value);
         }
 
+        /// <summary>
+        /// Gets a value indicating whether the scope has been initialized.
+        /// </summary>
+        public bool IsScopeCreated => initializer == null;
+
         /// <summary>
         /// Gets the scope if it has been initialized, else default.
         /// </summary>