Skip to content

Commit 463b249

Browse files
fredericDelaportefredericDelaporte
committed
Fix GetQueryCache building two different caches for same region.
1 parent 0549864 commit 463b249

File tree

6 files changed

+33
-107
lines changed

6 files changed

+33
-107
lines changed

‎src/NHibernate.Test/Async/CacheTest/GetQueryCacheFixture.cs‎

Lines changed: 2 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ public async Task RetrievedQueryCacheMatchesGloballyStoredOneAsync()
3737
{
3838
var region = "RetrievedQueryCacheMatchesGloballyStoredOne";
3939
LockedCache.Semaphore = new SemaphoreSlim(0);
40+
LockedCache.CreationCount = 0;
4041
try
4142
{
4243
var failures = new ConcurrentBag<Exception>();
@@ -89,44 +90,7 @@ public async Task RetrievedQueryCacheMatchesGloballyStoredOneAsync()
8990
var queryCache = Sfi.GetQueryCache(region).Cache;
9091
var globalCache = Sfi.GetSecondLevelCacheRegion(region);
9192
Assert.That(globalCache, Is.SameAs(queryCache));
93+
Assert.That(LockedCache.CreationCount, Is.EqualTo(1));
9294
}
9395
}
94-
95-
public partial class LockedCache : ICache
96-
{
97-
98-
#region Void implementation
99-
100-
public Task<object> GetAsync(object key, CancellationToken cancellationToken)
101-
{
102-
return Task.FromResult<object>(null);
103-
}
104-
105-
public Task PutAsync(object key, object value, CancellationToken cancellationToken)
106-
{
107-
return Task.CompletedTask;
108-
}
109-
110-
public Task RemoveAsync(object key, CancellationToken cancellationToken)
111-
{
112-
return Task.CompletedTask;
113-
}
114-
115-
public Task ClearAsync(CancellationToken cancellationToken)
116-
{
117-
return Task.CompletedTask;
118-
}
119-
120-
public Task LockAsync(object key, CancellationToken cancellationToken)
121-
{
122-
return Task.CompletedTask;
123-
}
124-
125-
public Task UnlockAsync(object key, CancellationToken cancellationToken)
126-
{
127-
return Task.CompletedTask;
128-
}
129-
130-
#endregion
131-
}
13296
}

‎src/NHibernate.Test/Async/SecondLevelCacheTest/InvalidationTests.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
using System.Reflection;
1515
using NHibernate.Cache;
1616
using NHibernate.Cfg;
17-
using NHibernate.Engine;
1817
using NHibernate.Impl;
1918
using NHibernate.Test.SecondLevelCacheTests;
2019
using NSubstitute;

‎src/NHibernate.Test/CacheTest/GetQueryCacheFixture.cs‎

Lines changed: 10 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public void RetrievedQueryCacheMatchesGloballyStoredOne()
2626
{
2727
var region = "RetrievedQueryCacheMatchesGloballyStoredOne";
2828
LockedCache.Semaphore = new SemaphoreSlim(0);
29+
LockedCache.CreationCount = 0;
2930
try
3031
{
3132
var failures = new ConcurrentBag<Exception>();
@@ -78,60 +79,27 @@ public void RetrievedQueryCacheMatchesGloballyStoredOne()
7879
var queryCache = Sfi.GetQueryCache(region).Cache;
7980
var globalCache = Sfi.GetSecondLevelCacheRegion(region);
8081
Assert.That(globalCache, Is.SameAs(queryCache));
82+
Assert.That(LockedCache.CreationCount, Is.EqualTo(1));
8183
}
8284
}
8385

84-
public partial class LockedCache : ICache
86+
public class LockedCache : HashtableCache
8587
{
8688
public static SemaphoreSlim Semaphore { get; set; }
8789

88-
public LockedCache(string regionName)
89-
{
90-
RegionName = regionName;
91-
Semaphore?.Wait();
92-
}
93-
94-
#region Void implementation
95-
96-
public object Get(object key)
97-
{
98-
return null;
99-
}
100-
101-
public void Put(object key, object value)
102-
{
103-
}
104-
105-
public void Remove(object key)
106-
{
107-
}
108-
109-
public void Clear()
110-
{
111-
}
112-
113-
public void Destroy()
114-
{
115-
}
116-
117-
public void Lock(object key)
118-
{
119-
}
90+
private static int _creationCount;
12091

121-
public void Unlock(object key)
92+
public static int CreationCount
12293
{
94+
get => _creationCount;
95+
set => _creationCount = value;
12396
}
12497

125-
public long NextTimestamp()
98+
public LockedCache(string regionName) : base(regionName)
12699
{
127-
return Timestamper.Next();
100+
Semaphore?.Wait();
101+
Interlocked.Increment(ref _creationCount);
128102
}
129-
130-
public int Timeout => Timestamper.OneMs * 60000;
131-
132-
public string RegionName { get; }
133-
134-
#endregion
135103
}
136104

137105
public class LockedCacheProvider : ICacheProvider

‎src/NHibernate/Async/Impl/SessionFactoryImpl.cs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,9 @@ public sealed partial class SessionFactoryImpl : ISessionFactoryImplementor, IOb
8686
{
8787
queryCache.Destroy();
8888

89-
foreach (IQueryCache cache in queryCaches.Values)
89+
foreach (var cache in queryCaches.Values)
9090
{
91-
cache.Destroy();
91+
cache.Value.Destroy();
9292
}
9393

9494
updateTimestampsCache.Destroy();
@@ -296,10 +296,9 @@ public sealed partial class SessionFactoryImpl : ISessionFactoryImplementor, IOb
296296
{
297297
if (settings.IsQueryCacheEnabled)
298298
{
299-
IQueryCache currentQueryCache;
300-
if (queryCaches.TryGetValue(cacheRegion, out currentQueryCache))
299+
if (queryCaches.TryGetValue(cacheRegion, out var currentQueryCache))
301300
{
302-
return currentQueryCache.ClearAsync(cancellationToken);
301+
return currentQueryCache.Value.ClearAsync(cancellationToken);
303302
}
304303
}
305304
}

‎src/NHibernate/Impl/SessionFactoryImpl.cs‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ public void HandleEntityNotFound(string entityName, object id)
148148
private readonly IQueryCache queryCache;
149149

150150
[NonSerialized]
151-
private readonly ConcurrentDictionary<string, IQueryCache> queryCaches;
151+
private readonly ConcurrentDictionary<string, Lazy<IQueryCache>> queryCaches;
152152
[NonSerialized]
153153
private readonly SchemaExport schemaExport;
154154
[NonSerialized]
@@ -379,7 +379,7 @@ public SessionFactoryImpl(Configuration cfg, IMapping mapping, Settings settings
379379
{
380380
updateTimestampsCache = new UpdateTimestampsCache(settings, properties);
381381
queryCache = settings.QueryCacheFactory.GetQueryCache(null, updateTimestampsCache, settings, properties);
382-
queryCaches = new ConcurrentDictionary<string, IQueryCache>();
382+
queryCaches = new ConcurrentDictionary<string, Lazy<IQueryCache>>();
383383
}
384384
else
385385
{
@@ -845,9 +845,9 @@ public void Close()
845845
{
846846
queryCache.Destroy();
847847

848-
foreach (IQueryCache cache in queryCaches.Values)
848+
foreach (var cache in queryCaches.Values)
849849
{
850-
cache.Destroy();
850+
cache.Value.Destroy();
851851
}
852852

853853
updateTimestampsCache.Destroy();
@@ -1027,21 +1027,18 @@ public IQueryCache GetQueryCache(string cacheRegion)
10271027
return null;
10281028
}
10291029

1030-
var wasAdded = false;
10311030
// The factory may be run concurrently by threads trying to get the same region.
1032-
// But the GetOrAdd will yield the same cache for all threads, so in case of add, use
1033-
// it to update allCacheRegions
1034-
var cache = queryCaches.GetOrAdd(
1031+
// But the GetOrAdd will yield the same lazy for all threads, so only one will
1032+
// initialize. https://stackoverflow.com/a/31637510/1178314
1033+
return queryCaches.GetOrAdd(
10351034
cacheRegion,
1036-
cr =>
1037-
{
1038-
var currentQueryCache = settings.QueryCacheFactory.GetQueryCache(cacheRegion, updateTimestampsCache, settings, properties);
1039-
wasAdded = true;
1040-
return currentQueryCache;
1041-
});
1042-
if (wasAdded)
1043-
allCacheRegions[cache.RegionName] = cache.Cache;
1044-
return cache;
1035+
cr => new Lazy<IQueryCache>(
1036+
() =>
1037+
{
1038+
var currentQueryCache = settings.QueryCacheFactory.GetQueryCache(cr, updateTimestampsCache, settings, properties);
1039+
allCacheRegions[currentQueryCache.RegionName] = currentQueryCache.Cache;
1040+
return currentQueryCache;
1041+
})).Value;
10451042
}
10461043

10471044
public void EvictQueries()
@@ -1067,10 +1064,9 @@ public void EvictQueries(string cacheRegion)
10671064
{
10681065
if (settings.IsQueryCacheEnabled)
10691066
{
1070-
IQueryCache currentQueryCache;
1071-
if (queryCaches.TryGetValue(cacheRegion, out currentQueryCache))
1067+
if (queryCaches.TryGetValue(cacheRegion, out var currentQueryCache))
10721068
{
1073-
currentQueryCache.Clear();
1069+
currentQueryCache.Value.Clear();
10741070
}
10751071
}
10761072
}

‎src/NHibernate/Util/StringHelper.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ public static string ReplaceOnce(string template, string placeholder, string rep
159159
}
160160

161161
/// <summary>
162-
/// Just a façade for calling string.Split()
162+
/// Just a facade for calling string.Split()
163163
/// We don't use our StringTokenizer because string.Split() is
164164
/// more efficient (but it only works when we don't want to retrieve the delimiters)
165165
/// </summary>

0 commit comments

Comments
 (0)