Skip to content

Commit 4f528b3

Browse files
bitfasterbitfaster
authored
Atomic caches are in an inconsistent state when factory throws (#324)
* poc count * comments * fix key enum * comments * cleanup * cleanup
1 parent 3b9baff commit 4f528b3

11 files changed

+223
-14
lines changed

‎BitFaster.Caching.UnitTests/Atomic/AtomicFactoryAsyncCacheTests.cs‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,43 @@ public void WhenItemsAddedEnumerateContainsKvps()
203203
enumerable.Should().BeEquivalentTo(new[] { new KeyValuePair<int, int>(1, 1), new KeyValuePair<int, int>(2, 2) });
204204
}
205205

206+
[Fact]
207+
public async Task WhenFactoryThrowsEmptyValueIsNotCounted()
208+
{
209+
try
210+
{
211+
await cache.GetOrAddAsync(1, k => throw new ArithmeticException());
212+
}
213+
catch { }
214+
215+
cache.Count.Should().Be(0);
216+
}
217+
218+
[Fact]
219+
public async Task WhenFactoryThrowsEmptyValueIsNotEnumerable()
220+
{
221+
try
222+
{
223+
await cache.GetOrAddAsync(1, k => throw new ArithmeticException());
224+
}
225+
catch { }
226+
227+
// IEnumerable.Count() instead of Count property
228+
cache.Count().Should().Be(0);
229+
}
230+
231+
[Fact]
232+
public async Task WhenFactoryThrowsEmptyKeyIsNotEnumerable()
233+
{
234+
try
235+
{
236+
await cache.GetOrAddAsync(1, k => throw new ArithmeticException());
237+
}
238+
catch { }
239+
240+
cache.Keys.Count().Should().Be(0);
241+
}
242+
206243
private void OnItemRemoved(object sender, ItemRemovedEventArgs<int, int> e)
207244
{
208245
this.removedItems.Add(e);

‎BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheTests.cs‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,43 @@ public void WhenItemsAddedEnumerateContainsKvps()
203203
enumerable.Should().BeEquivalentTo(new[] { new KeyValuePair<int, int>(1, 1), new KeyValuePair<int, int>(2, 2) });
204204
}
205205

206+
[Fact]
207+
public void WhenFactoryThrowsEmptyValueIsNotCounted()
208+
{
209+
try
210+
{
211+
cache.GetOrAdd(1, _ => throw new Exception());
212+
}
213+
catch { }
214+
215+
cache.Count.Should().Be(0);
216+
}
217+
218+
[Fact]
219+
public void WhenFactoryThrowsEmptyValueIsNotEnumerable()
220+
{
221+
try
222+
{
223+
cache.GetOrAdd(1, k => throw new Exception());
224+
}
225+
catch { }
226+
227+
// IEnumerable.Count() instead of Count property
228+
cache.Count().Should().Be(0);
229+
}
230+
231+
[Fact]
232+
public void WhenFactoryThrowsEmptyKeyIsNotEnumerable()
233+
{
234+
try
235+
{
236+
cache.GetOrAdd(1, k => throw new Exception());
237+
}
238+
catch { }
239+
240+
cache.Keys.Count().Should().Be(0);
241+
}
242+
206243
private void OnItemRemoved(object sender, ItemRemovedEventArgs<int, int> e)
207244
{
208245
this.removedItems.Add(e);

‎BitFaster.Caching.UnitTests/Atomic/AtomicFactoryScopedCacheTests.cs‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,5 +83,42 @@ public void WhenEntryIsUpdatedOldEntryIsDisposed()
8383
this.cache.TryUpdate(1, new Disposable()).Should().BeTrue();
8484
disposable2.IsDisposed.Should().BeTrue();
8585
}
86+
87+
[Fact]
88+
public void WhenFactoryThrowsEmptyValueIsNotCounted()
89+
{
90+
try
91+
{
92+
cache.ScopedGetOrAdd(1, _ => throw new Exception());
93+
}
94+
catch { }
95+
96+
cache.Count.Should().Be(0);
97+
}
98+
99+
[Fact]
100+
public void WhenFactoryThrowsEmptyValueIsNotEnumerable()
101+
{
102+
try
103+
{
104+
cache.ScopedGetOrAdd(1, k => throw new Exception());
105+
}
106+
catch { }
107+
108+
// IEnumerable.Count() instead of Count property
109+
cache.Count().Should().Be(0);
110+
}
111+
112+
[Fact]
113+
public void WhenFactoryThrowsEmptyKeyIsNotEnumerable()
114+
{
115+
try
116+
{
117+
cache.ScopedGetOrAdd(1, k => throw new Exception());
118+
}
119+
catch { }
120+
121+
cache.Keys.Count().Should().Be(0);
122+
}
86123
}
87124
}

‎BitFaster.Caching.UnitTests/ScopedAsyncCacheTestBase.cs‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,43 @@ public void WhenItemsAddedEnumerateContainsKvps()
228228
list.Should().BeEquivalentTo(new[] { new KeyValuePair<int, Disposable>(1, d1), new KeyValuePair<int, Disposable>(2, d2) });
229229
}
230230

231+
[Fact]
232+
public async Task WhenFactoryThrowsEmptyValueIsNotCounted()
233+
{
234+
try
235+
{
236+
await cache.ScopedGetOrAddAsync(1, k => throw new ArithmeticException());
237+
}
238+
catch { }
239+
240+
cache.Count.Should().Be(0);
241+
}
242+
243+
[Fact]
244+
public async Task WhenFactoryThrowsEmptyValueIsNotEnumerable()
245+
{
246+
try
247+
{
248+
await cache.ScopedGetOrAddAsync(1, k => throw new ArithmeticException());
249+
}
250+
catch { }
251+
252+
// IEnumerable.Count() instead of Count property
253+
cache.Count().Should().Be(0);
254+
}
255+
256+
[Fact]
257+
public async Task WhenFactoryThrowsEmptyKeyIsNotEnumerable()
258+
{
259+
try
260+
{
261+
await cache.ScopedGetOrAddAsync(1, k => throw new ArithmeticException());
262+
}
263+
catch { }
264+
265+
cache.Keys.Count().Should().Be(0);
266+
}
267+
231268
protected void OnItemRemoved(object sender, ItemRemovedEventArgs<int, Scoped<Disposable>> e)
232269
{
233270
this.removedItems.Add(e);

‎BitFaster.Caching/Atomic/AtomicEx.cs‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
using System;
2+
using System.Collections;
3+
using System.Collections.Generic;
4+
using System.Collections.ObjectModel;
5+
using System.Linq;
6+
using System.Text;
7+
8+
namespace BitFaster.Caching.Atomic
9+
{
10+
internal static class AtomicEx
11+
{
12+
internal static int EnumerateCount(IEnumerator enumerator)
13+
{
14+
int count = 0;
15+
while (enumerator.MoveNext())
16+
{
17+
count++;
18+
}
19+
return count;
20+
}
21+
22+
internal static ICollection<K> FilterKeys<K, V>(IEnumerable<KeyValuePair<K, V>> kvps, Func<V, bool> filter)
23+
{
24+
// Here we will double enumerate the kvps list. Alternative is to lazy init the size which will keep resizing
25+
// the List, and spam allocs if the list is long.
26+
List<K> keys = new List<K>(kvps.Count());
27+
28+
foreach (var kvp in kvps)
29+
{
30+
if (filter(kvp.Value))
31+
{
32+
keys.Add(kvp.Key);
33+
}
34+
}
35+
36+
return new ReadOnlyCollection<K>(keys);
37+
}
38+
}
39+
}

‎BitFaster.Caching/Atomic/AtomicFactoryAsyncCache.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public AtomicFactoryAsyncCache(ICache<K, AsyncAtomicFactory<K, V>> cache)
4343
}
4444

4545
///<inheritdoc/>
46-
public int Count => cache.Count;
46+
public int Count => AtomicEx.EnumerateCount(this.GetEnumerator());
4747

4848
///<inheritdoc/>
4949
public Optional<ICacheMetrics> Metrics => cache.Metrics;
@@ -52,7 +52,7 @@ public AtomicFactoryAsyncCache(ICache<K, AsyncAtomicFactory<K, V>> cache)
5252
public Optional<ICacheEvents<K, V>> Events => this.events;
5353

5454
///<inheritdoc/>
55-
public ICollection<K> Keys => this.cache.Keys;
55+
public ICollection<K> Keys => AtomicEx.FilterKeys<K, AsyncAtomicFactory<K, V>>(this.cache, v => v.IsValueCreated);
5656

5757
///<inheritdoc/>
5858
public CachePolicy Policy => this.cache.Policy;
@@ -109,7 +109,10 @@ public IEnumerator<KeyValuePair<K, V>> GetEnumerator()
109109
{
110110
foreach (var kvp in this.cache)
111111
{
112-
yield return new KeyValuePair<K, V>(kvp.Key, kvp.Value.ValueIfCreated);
112+
if (kvp.Value.IsValueCreated)
113+
{
114+
yield return new KeyValuePair<K, V>(kvp.Key, kvp.Value.ValueIfCreated);
115+
}
113116
}
114117
}
115118

‎BitFaster.Caching/Atomic/AtomicFactoryCache.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public AtomicFactoryCache(ICache<K, AtomicFactory<K, V>> cache)
4141
}
4242

4343
///<inheritdoc/>
44-
public int Count => this.cache.Count;
44+
public int Count => AtomicEx.EnumerateCount(this.GetEnumerator());
4545

4646
///<inheritdoc/>
4747
public Optional<ICacheMetrics> Metrics => this.cache.Metrics;
@@ -50,7 +50,7 @@ public AtomicFactoryCache(ICache<K, AtomicFactory<K, V>> cache)
5050
public Optional<ICacheEvents<K, V>> Events => this.events;
5151

5252
///<inheritdoc/>
53-
public ICollection<K> Keys => this.cache.Keys;
53+
public ICollection<K> Keys => AtomicEx.FilterKeys<K, AtomicFactory<K, V>>(this.cache, v => v.IsValueCreated);
5454

5555
///<inheritdoc/>
5656
public CachePolicy Policy => this.cache.Policy;
@@ -107,7 +107,10 @@ public IEnumerator<KeyValuePair<K, V>> GetEnumerator()
107107
{
108108
foreach (var kvp in this.cache)
109109
{
110-
yield return new KeyValuePair<K, V>(kvp.Key, kvp.Value.ValueIfCreated);
110+
if (kvp.Value.IsValueCreated)
111+
{
112+
yield return new KeyValuePair<K, V>(kvp.Key, kvp.Value.ValueIfCreated);
113+
}
111114
}
112115
}
113116

‎BitFaster.Caching/Atomic/AtomicFactoryScopedAsyncCache.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public AtomicFactoryScopedAsyncCache(ICache<K, ScopedAsyncAtomicFactory<K, V>> c
4242
}
4343

4444
///<inheritdoc/>
45-
public int Count => this.cache.Count;
45+
public int Count => AtomicEx.EnumerateCount(this.GetEnumerator());
4646

4747
///<inheritdoc/>
4848
public Optional<ICacheMetrics> Metrics => this.cache.Metrics;
@@ -54,7 +54,7 @@ public AtomicFactoryScopedAsyncCache(ICache<K, ScopedAsyncAtomicFactory<K, V>> c
5454
public CachePolicy Policy => this.cache.Policy;
5555

5656
///<inheritdoc/>
57-
public ICollection<K> Keys => this.cache.Keys;
57+
public ICollection<K> Keys => AtomicEx.FilterKeys<K, ScopedAsyncAtomicFactory<K, V>>(this.cache, v => v.IsScopeCreated);
5858

5959
///<inheritdoc/>
6060
public void AddOrUpdate(K key, V value)
@@ -125,7 +125,10 @@ public IEnumerator<KeyValuePair<K, Scoped<V>>> GetEnumerator()
125125
{
126126
foreach (var kvp in this.cache)
127127
{
128-
yield return new KeyValuePair<K, Scoped<V>>(kvp.Key, kvp.Value.ScopeIfCreated);
128+
if (kvp.Value.IsScopeCreated)
129+
{
130+
yield return new KeyValuePair<K, Scoped<V>>(kvp.Key, kvp.Value.ScopeIfCreated);
131+
}
129132
}
130133
}
131134

‎BitFaster.Caching/Atomic/AtomicFactoryScopedCache.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public AtomicFactoryScopedCache(ICache<K, ScopedAtomicFactory<K, V>> cache)
4242
}
4343

4444
///<inheritdoc/>
45-
public int Count => this.cache.Count;
45+
public int Count => AtomicEx.EnumerateCount(this.GetEnumerator());
4646

4747
///<inheritdoc/>
4848
public Optional<ICacheMetrics> Metrics => this.cache.Metrics;
@@ -54,7 +54,7 @@ public AtomicFactoryScopedCache(ICache<K, ScopedAtomicFactory<K, V>> cache)
5454
public CachePolicy Policy => this.cache.Policy;
5555

5656
///<inheritdoc/>
57-
public ICollection<K> Keys => this.cache.Keys;
57+
public ICollection<K> Keys => AtomicEx.FilterKeys<K, ScopedAtomicFactory<K, V>>(this.cache, v => v.IsScopeCreated);
5858

5959
///<inheritdoc/>
6060
public void AddOrUpdate(K key, V value)
@@ -123,7 +123,10 @@ public IEnumerator<KeyValuePair<K, Scoped<V>>> GetEnumerator()
123123
{
124124
foreach (var kvp in this.cache)
125125
{
126-
yield return new KeyValuePair<K, Scoped<V>>(kvp.Key, kvp.Value.ScopeIfCreated);
126+
if (kvp.Value.IsScopeCreated)
127+
{
128+
yield return new KeyValuePair<K, Scoped<V>>(kvp.Key, kvp.Value.ScopeIfCreated);
129+
}
127130
}
128131
}
129132

‎BitFaster.Caching/Atomic/ScopedAsyncAtomicFactory.cs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ namespace BitFaster.Caching.Atomic
1111
/// </summary>
1212
/// <typeparam name="K">The type of the key.</typeparam>
1313
/// <typeparam name="V">The type of the value.</typeparam>
14-
[DebuggerDisplay("IsValueCreated={initializer == null}, Value={ScopeIfCreated}")]
14+
[DebuggerDisplay("IsScopeCreated={initializer == null}, Value={ScopeIfCreated}")]
1515
public sealed class ScopedAsyncAtomicFactory<K, V> : IScoped<V>, IDisposable where V : IDisposable
1616
{
1717
private Scoped<V> scope;
@@ -35,6 +35,11 @@ public ScopedAsyncAtomicFactory(V value)
3535
scope = new Scoped<V>(value);
3636
}
3737

38+
/// <summary>
39+
/// Gets a value indicating whether the scope has been initialized.
40+
/// </summary>
41+
public bool IsScopeCreated => initializer == null;
42+
3843
/// <summary>
3944
/// Gets the scope if it has been initialized, else default.
4045
/// </summary>

0 commit comments

Comments
 (0)