Skip to content

Commit 10af142

Browse files
fredericDelaportefredericDelaporte
committed
Fix GetQueryCache building and storing two different caches
1 parent 73797df commit 10af142

File tree

6 files changed

+242
-20
lines changed

6 files changed

+242
-20
lines changed

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

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
//------------------------------------------------------------------------------
2+
// <auto-generated>
3+
// This code was generated by AsyncGenerator.
4+
//
5+
// Changes to this file may cause incorrect behavior and will be lost if
6+
// the code is regenerated.
7+
// </auto-generated>
8+
//------------------------------------------------------------------------------
9+
10+
11+
using System;
12+
using System.Collections;
13+
using System.Collections.Concurrent;
14+
using System.Collections.Generic;
15+
using System.Threading;
16+
using NHibernate.Cache;
17+
using NHibernate.Cfg;
18+
using NUnit.Framework;
19+
using Environment = NHibernate.Cfg.Environment;
20+
21+
namespace NHibernate.Test.CacheTest
22+
{
23+
using System.Threading.Tasks;
24+
[TestFixture]
25+
public class GetQueryCacheFixtureAsync : TestCase
26+
{
27+
protected override IList Mappings => new[] { "Simple.hbm.xml" };
28+
29+
protected override void Configure(Configuration configuration)
30+
{
31+
configuration.SetProperty(Environment.UseQueryCache, "true");
32+
configuration.SetProperty(Environment.CacheProvider, typeof(LockedCacheProvider).AssemblyQualifiedName);
33+
}
34+
35+
[Test]
36+
public async Task RetrievedQueryCacheMatchesGloballyStoredOneAsync()
37+
{
38+
var region = "RetrievedQueryCacheMatchesGloballyStoredOne";
39+
LockedCache.Semaphore = new SemaphoreSlim(0);
40+
LockedCache.CreationCount = 0;
41+
try
42+
{
43+
var failures = new ConcurrentBag<Exception>();
44+
var thread1 = new Thread(
45+
() =>
46+
{
47+
try
48+
{
49+
Sfi.GetQueryCache(region);
50+
}
51+
catch (Exception e)
52+
{
53+
failures.Add(e);
54+
}
55+
});
56+
var thread2 = new Thread(
57+
() =>
58+
{
59+
try
60+
{
61+
Sfi.GetQueryCache(region);
62+
}
63+
catch (Exception e)
64+
{
65+
failures.Add(e);
66+
}
67+
});
68+
thread1.Start();
69+
thread2.Start();
70+
71+
// Give some time to threads for reaching the wait, having all of them ready to do most of their job concurrently.
72+
await (Task.Delay(100));
73+
// Let only one finish its job, it should be the one being stored in query cache dictionary.
74+
LockedCache.Semaphore.Release(1);
75+
// Give some time to released thread to finish its job.
76+
await (Task.Delay(100));
77+
// Release other thread
78+
LockedCache.Semaphore.Release(10);
79+
80+
thread1.Join();
81+
thread2.Join();
82+
Assert.That(failures, Is.Empty, $"{failures.Count} thread(s) failed.");
83+
}
84+
finally
85+
{
86+
LockedCache.Semaphore.Dispose();
87+
LockedCache.Semaphore = null;
88+
}
89+
90+
var queryCache = Sfi.GetQueryCache(region).Cache;
91+
var globalCache = Sfi.GetSecondLevelCacheRegion(region);
92+
Assert.That(globalCache, Is.SameAs(queryCache));
93+
Assert.That(LockedCache.CreationCount, Is.EqualTo(1));
94+
}
95+
}
96+
}

‎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: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
using System;
2+
using System.Collections;
3+
using System.Collections.Concurrent;
4+
using System.Collections.Generic;
5+
using System.Threading;
6+
using NHibernate.Cache;
7+
using NHibernate.Cfg;
8+
using NUnit.Framework;
9+
using Environment = NHibernate.Cfg.Environment;
10+
11+
namespace NHibernate.Test.CacheTest
12+
{
13+
[TestFixture]
14+
public class GetQueryCacheFixture : TestCase
15+
{
16+
protected override IList Mappings => new[] { "Simple.hbm.xml" };
17+
18+
protected override void Configure(Configuration configuration)
19+
{
20+
configuration.SetProperty(Environment.UseQueryCache, "true");
21+
configuration.SetProperty(Environment.CacheProvider, typeof(LockedCacheProvider).AssemblyQualifiedName);
22+
}
23+
24+
[Test]
25+
public void RetrievedQueryCacheMatchesGloballyStoredOne()
26+
{
27+
var region = "RetrievedQueryCacheMatchesGloballyStoredOne";
28+
LockedCache.Semaphore = new SemaphoreSlim(0);
29+
LockedCache.CreationCount = 0;
30+
try
31+
{
32+
var failures = new ConcurrentBag<Exception>();
33+
var thread1 = new Thread(
34+
() =>
35+
{
36+
try
37+
{
38+
Sfi.GetQueryCache(region);
39+
}
40+
catch (Exception e)
41+
{
42+
failures.Add(e);
43+
}
44+
});
45+
var thread2 = new Thread(
46+
() =>
47+
{
48+
try
49+
{
50+
Sfi.GetQueryCache(region);
51+
}
52+
catch (Exception e)
53+
{
54+
failures.Add(e);
55+
}
56+
});
57+
thread1.Start();
58+
thread2.Start();
59+
60+
// Give some time to threads for reaching the wait, having all of them ready to do most of their job concurrently.
61+
Thread.Sleep(100);
62+
// Let only one finish its job, it should be the one being stored in query cache dictionary.
63+
LockedCache.Semaphore.Release(1);
64+
// Give some time to released thread to finish its job.
65+
Thread.Sleep(100);
66+
// Release other thread
67+
LockedCache.Semaphore.Release(10);
68+
69+
thread1.Join();
70+
thread2.Join();
71+
Assert.That(failures, Is.Empty, $"{failures.Count} thread(s) failed.");
72+
}
73+
finally
74+
{
75+
LockedCache.Semaphore.Dispose();
76+
LockedCache.Semaphore = null;
77+
}
78+
79+
var queryCache = Sfi.GetQueryCache(region).Cache;
80+
var globalCache = Sfi.GetSecondLevelCacheRegion(region);
81+
Assert.That(globalCache, Is.SameAs(queryCache));
82+
Assert.That(LockedCache.CreationCount, Is.EqualTo(1));
83+
}
84+
}
85+
86+
public class LockedCache : HashtableCache
87+
{
88+
public static SemaphoreSlim Semaphore { get; set; }
89+
90+
private static int _creationCount;
91+
92+
public static int CreationCount
93+
{
94+
get => _creationCount;
95+
set => _creationCount = value;
96+
}
97+
98+
public LockedCache(string regionName) : base(regionName)
99+
{
100+
Semaphore?.Wait();
101+
Interlocked.Increment(ref _creationCount);
102+
}
103+
}
104+
105+
public class LockedCacheProvider : ICacheProvider
106+
{
107+
public ICache BuildCache(string regionName, IDictionary<string, string> properties)
108+
{
109+
return new LockedCache(regionName);
110+
}
111+
112+
public long NextTimestamp()
113+
{
114+
return Timestamper.Next();
115+
}
116+
117+
public void Start(IDictionary<string, string> properties)
118+
{
119+
}
120+
121+
public void Stop()
122+
{
123+
}
124+
}
125+
}

‎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 & 13 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,14 +1027,18 @@ public IQueryCache GetQueryCache(string cacheRegion)
10271027
return null;
10281028
}
10291029

1030+
// The factory may be run concurrently by threads trying to get the same region.
1031+
// But the GetOrAdd will yield the same lazy for all threads, so only one will
1032+
// initialize. https://stackoverflow.com/a/31637510/1178314
10301033
return queryCaches.GetOrAdd(
10311034
cacheRegion,
1032-
cr =>
1033-
{
1034-
IQueryCache currentQueryCache = settings.QueryCacheFactory.GetQueryCache(cacheRegion, updateTimestampsCache, settings, properties);
1035-
allCacheRegions[currentQueryCache.RegionName] = currentQueryCache.Cache;
1036-
return currentQueryCache;
1037-
});
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;
10381042
}
10391043

10401044
public void EvictQueries()
@@ -1060,10 +1064,9 @@ public void EvictQueries(string cacheRegion)
10601064
{
10611065
if (settings.IsQueryCacheEnabled)
10621066
{
1063-
IQueryCache currentQueryCache;
1064-
if (queryCaches.TryGetValue(cacheRegion, out currentQueryCache))
1067+
if (queryCaches.TryGetValue(cacheRegion, out var currentQueryCache))
10651068
{
1066-
currentQueryCache.Clear();
1069+
currentQueryCache.Value.Clear();
10671070
}
10681071
}
10691072
}

‎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)