diff --git a/src/NHibernate.Test/Async/CacheTest/GetQueryCacheFixture.cs b/src/NHibernate.Test/Async/CacheTest/GetQueryCacheFixture.cs new file mode 100644 index 00000000000..0efbe55d3e2 --- /dev/null +++ b/src/NHibernate.Test/Async/CacheTest/GetQueryCacheFixture.cs @@ -0,0 +1,96 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by AsyncGenerator. +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + + +using System; +using System.Collections; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Threading; +using NHibernate.Cache; +using NHibernate.Cfg; +using NUnit.Framework; +using Environment = NHibernate.Cfg.Environment; + +namespace NHibernate.Test.CacheTest +{ + using System.Threading.Tasks; + [TestFixture] + public class GetQueryCacheFixtureAsync : TestCase + { + protected override IList Mappings => new[] { "Simple.hbm.xml" }; + + protected override void Configure(Configuration configuration) + { + configuration.SetProperty(Environment.UseQueryCache, "true"); + configuration.SetProperty(Environment.CacheProvider, typeof(LockedCacheProvider).AssemblyQualifiedName); + } + + [Test] + public async Task RetrievedQueryCacheMatchesGloballyStoredOneAsync() + { + var region = "RetrievedQueryCacheMatchesGloballyStoredOne"; + LockedCache.Semaphore = new SemaphoreSlim(0); + LockedCache.CreationCount = 0; + try + { + var failures = new ConcurrentBag(); + var thread1 = new Thread( + () => + { + try + { + Sfi.GetQueryCache(region); + } + catch (Exception e) + { + failures.Add(e); + } + }); + var thread2 = new Thread( + () => + { + try + { + Sfi.GetQueryCache(region); + } + catch (Exception e) + { + failures.Add(e); + } + }); + thread1.Start(); + thread2.Start(); + + // Give some time to threads for reaching the wait, having all of them ready to do most of their job concurrently. + await (Task.Delay(100)); + // Let only one finish its job, it should be the one being stored in query cache dictionary. + LockedCache.Semaphore.Release(1); + // Give some time to released thread to finish its job. + await (Task.Delay(100)); + // Release other thread + LockedCache.Semaphore.Release(10); + + thread1.Join(); + thread2.Join(); + Assert.That(failures, Is.Empty, $"{failures.Count} thread(s) failed."); + } + finally + { + LockedCache.Semaphore.Dispose(); + LockedCache.Semaphore = null; + } + + var queryCache = Sfi.GetQueryCache(region).Cache; + var globalCache = Sfi.GetSecondLevelCacheRegion(region); + Assert.That(globalCache, Is.SameAs(queryCache)); + Assert.That(LockedCache.CreationCount, Is.EqualTo(1)); + } + } +} diff --git a/src/NHibernate.Test/CacheTest/GetQueryCacheFixture.cs b/src/NHibernate.Test/CacheTest/GetQueryCacheFixture.cs new file mode 100644 index 00000000000..8e5c1e4c938 --- /dev/null +++ b/src/NHibernate.Test/CacheTest/GetQueryCacheFixture.cs @@ -0,0 +1,125 @@ +using System; +using System.Collections; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Threading; +using NHibernate.Cache; +using NHibernate.Cfg; +using NUnit.Framework; +using Environment = NHibernate.Cfg.Environment; + +namespace NHibernate.Test.CacheTest +{ + [TestFixture] + public class GetQueryCacheFixture : TestCase + { + protected override IList Mappings => new[] { "Simple.hbm.xml" }; + + protected override void Configure(Configuration configuration) + { + configuration.SetProperty(Environment.UseQueryCache, "true"); + configuration.SetProperty(Environment.CacheProvider, typeof(LockedCacheProvider).AssemblyQualifiedName); + } + + [Test] + public void RetrievedQueryCacheMatchesGloballyStoredOne() + { + var region = "RetrievedQueryCacheMatchesGloballyStoredOne"; + LockedCache.Semaphore = new SemaphoreSlim(0); + LockedCache.CreationCount = 0; + try + { + var failures = new ConcurrentBag(); + var thread1 = new Thread( + () => + { + try + { + Sfi.GetQueryCache(region); + } + catch (Exception e) + { + failures.Add(e); + } + }); + var thread2 = new Thread( + () => + { + try + { + Sfi.GetQueryCache(region); + } + catch (Exception e) + { + failures.Add(e); + } + }); + thread1.Start(); + thread2.Start(); + + // Give some time to threads for reaching the wait, having all of them ready to do most of their job concurrently. + Thread.Sleep(100); + // Let only one finish its job, it should be the one being stored in query cache dictionary. + LockedCache.Semaphore.Release(1); + // Give some time to released thread to finish its job. + Thread.Sleep(100); + // Release other thread + LockedCache.Semaphore.Release(10); + + thread1.Join(); + thread2.Join(); + Assert.That(failures, Is.Empty, $"{failures.Count} thread(s) failed."); + } + finally + { + LockedCache.Semaphore.Dispose(); + LockedCache.Semaphore = null; + } + + var queryCache = Sfi.GetQueryCache(region).Cache; + var globalCache = Sfi.GetSecondLevelCacheRegion(region); + Assert.That(globalCache, Is.SameAs(queryCache)); + Assert.That(LockedCache.CreationCount, Is.EqualTo(1)); + } + } + + public class LockedCache : HashtableCache + { + public static SemaphoreSlim Semaphore { get; set; } + + private static int _creationCount; + + public static int CreationCount + { + get => _creationCount; + set => _creationCount = value; + } + + public LockedCache(string regionName) : base(regionName) + { + Semaphore?.Wait(); + Interlocked.Increment(ref _creationCount); + } + } + + public class LockedCacheProvider : ICacheProvider + { + public ICache BuildCache(string regionName, IDictionary properties) + { + return new LockedCache(regionName); + } + + public long NextTimestamp() + { + return Timestamper.Next(); + } + + public void Start(IDictionary properties) + { + } + + public void Stop() + { + } + } +} diff --git a/src/NHibernate/Async/Impl/SessionFactoryImpl.cs b/src/NHibernate/Async/Impl/SessionFactoryImpl.cs index a82826eae6c..05b23b537ea 100644 --- a/src/NHibernate/Async/Impl/SessionFactoryImpl.cs +++ b/src/NHibernate/Async/Impl/SessionFactoryImpl.cs @@ -87,9 +87,9 @@ public sealed partial class SessionFactoryImpl : ISessionFactoryImplementor, IOb { queryCache.Destroy(); - foreach (IQueryCache cache in queryCaches.Values) + foreach (var cache in queryCaches.Values) { - cache.Destroy(); + cache.Value.Destroy(); } updateTimestampsCache.Destroy(); @@ -297,10 +297,9 @@ public sealed partial class SessionFactoryImpl : ISessionFactoryImplementor, IOb { if (settings.IsQueryCacheEnabled) { - IQueryCache currentQueryCache; - if (queryCaches.TryGetValue(cacheRegion, out currentQueryCache)) + if (queryCaches.TryGetValue(cacheRegion, out var currentQueryCache)) { - return currentQueryCache.ClearAsync(cancellationToken); + return currentQueryCache.Value.ClearAsync(cancellationToken); } } } diff --git a/src/NHibernate/Impl/SessionFactoryImpl.cs b/src/NHibernate/Impl/SessionFactoryImpl.cs index 78514bffb33..8c75b4e94a3 100644 --- a/src/NHibernate/Impl/SessionFactoryImpl.cs +++ b/src/NHibernate/Impl/SessionFactoryImpl.cs @@ -149,7 +149,7 @@ public void HandleEntityNotFound(string entityName, object id) private readonly IQueryCache queryCache; [NonSerialized] - private readonly ConcurrentDictionary queryCaches; + private readonly ConcurrentDictionary> queryCaches; [NonSerialized] private readonly SchemaExport schemaExport; [NonSerialized] @@ -381,7 +381,7 @@ public SessionFactoryImpl(Configuration cfg, IMapping mapping, Settings settings { updateTimestampsCache = new UpdateTimestampsCache(settings, properties); queryCache = settings.QueryCacheFactory.GetQueryCache(null, updateTimestampsCache, settings, properties); - queryCaches = new ConcurrentDictionary(); + queryCaches = new ConcurrentDictionary>(); } else { @@ -847,9 +847,9 @@ public void Close() { queryCache.Destroy(); - foreach (IQueryCache cache in queryCaches.Values) + foreach (var cache in queryCaches.Values) { - cache.Destroy(); + cache.Value.Destroy(); } updateTimestampsCache.Destroy(); @@ -1029,14 +1029,18 @@ public IQueryCache GetQueryCache(string cacheRegion) return null; } + // The factory may be run concurrently by threads trying to get the same region. + // But the GetOrAdd will yield the same lazy for all threads, so only one will + // initialize. https://stackoverflow.com/a/31637510/1178314 return queryCaches.GetOrAdd( cacheRegion, - cr => - { - IQueryCache currentQueryCache = settings.QueryCacheFactory.GetQueryCache(cacheRegion, updateTimestampsCache, settings, properties); - allCacheRegions[currentQueryCache.RegionName] = currentQueryCache.Cache; - return currentQueryCache; - }); + cr => new Lazy( + () => + { + var currentQueryCache = settings.QueryCacheFactory.GetQueryCache(cr, updateTimestampsCache, settings, properties); + allCacheRegions[currentQueryCache.RegionName] = currentQueryCache.Cache; + return currentQueryCache; + })).Value; } public void EvictQueries() @@ -1062,10 +1066,9 @@ public void EvictQueries(string cacheRegion) { if (settings.IsQueryCacheEnabled) { - IQueryCache currentQueryCache; - if (queryCaches.TryGetValue(cacheRegion, out currentQueryCache)) + if (queryCaches.TryGetValue(cacheRegion, out var currentQueryCache)) { - currentQueryCache.Clear(); + currentQueryCache.Value.Clear(); } } }