Handle CacheBase.Destroy in the session factory

Author: fredericDelaporte

src/NHibernate/Async/Impl/SessionFactoryImpl.cs

@@ -89,8 +89,11 @@ public sealed partial class SessionFactoryImpl : ISessionFactoryImplementor, IOb
 				{
 					cache.Value.Destroy();
 				}
+			}
 
-				updateTimestampsCache.Destroy();
+			foreach (var cache in _allCacheRegions.Values)
+			{
+				cache.Destroy();
 			}
 
 			settings.CacheProvider.Stop();

src/NHibernate/Cache/CacheFactory.cs

@@ -43,7 +43,7 @@ public static ICacheConcurrencyStrategy CreateCache(
 
 			var cache = BuildCacheBase(name, settings, properties);
 
-			var ccs = CreateCache(usage,cache);
+			var ccs = CreateCache(usage, cache);
 
 			if (mutable && usage == ReadOnly)
 				log.Warn("read-only cache configured for mutable: {0}", name);

src/NHibernate/Cache/ICacheConcurrencyStrategy.cs

@@ -124,9 +124,12 @@ public partial interface ICacheConcurrencyStrategy
 		void Clear();
 
 		/// <summary>
-		/// Clean up all resources.
+		/// Clean up resources.
 		/// </summary>
 		/// <exception cref="CacheException"></exception>
+		/// <remarks>
+		/// This method should not destroy <see cref="Cache" />. The session factory is responsible for it.
+		/// </remarks>
 		void Destroy();
 
 		/// <summary>

src/NHibernate/Cache/IQueryCache.cs

@@ -44,8 +44,11 @@ public partial interface IQueryCache
 		IList Get(QueryKey key, ICacheAssembler[] returnTypes, bool isNaturalKeyLookup, ISet<string> spaces, ISessionImplementor session);
 
 		/// <summary>
-		/// Clean up all resources.
+		/// Clean up resources.
 		/// </summary>
+		/// <remarks>
+		/// This method should not destroy <see cref="Cache" />. The session factory is responsible for it.
+		/// </remarks>
 		void Destroy();
 	}
 

src/NHibernate/Cache/NonstrictReadWriteCache.cs

@@ -203,14 +203,9 @@ public void Clear()
 
 		public void Destroy()
 		{
-			try
-			{
-				Cache.Destroy();
-			}
-			catch (Exception e)
-			{
-				log.Warn(e, "Could not destroy cache");
-			}
+			// The cache is externally provided and may be shared. Destroying the cache is
+			// not the responsibility of this class.
+			Cache = null;
 		}
 
 		/// <summary>

src/NHibernate/Cache/ReadOnlyCache.cs

@@ -181,14 +181,9 @@ public void Remove(CacheKey key)
 
 		public void Destroy()
 		{
-			try
-			{
-				Cache.Destroy();
-			}
-			catch (Exception e)
-			{
-				log.Warn(e, "Could not destroy cache");
-			}
+			// The cache is externally provided and may be shared. Destroying the cache is
+			// not the responsibility of this class.
+			Cache = null;
 		}
 
 		/// <summary>

src/NHibernate/Cache/ReadWriteCache.cs

@@ -413,14 +413,9 @@ public void Remove(CacheKey key)
 
 		public void Destroy()
 		{
-			try
-			{
-				Cache.Destroy();
-			}
-			catch (Exception e)
-			{
-				log.Warn(e, "Could not destroy cache");
-			}
+			// The cache is externally provided and may be shared. Destroying the cache is
+			// not the responsibility of this class.
+			Cache = null;
 		}
 
 		/// <summary>

src/NHibernate/Cache/StandardQueryCache.cs

@@ -281,14 +281,8 @@ public IList[] GetMany(
 
 		public void Destroy()
 		{
-			try
-			{
-				Cache.Destroy();
-			}
-			catch (Exception e)
-			{
-				Log.Warn(e, "could not destroy query cache: {0}", _regionName);
-			}
+			// The cache is externally provided and may be shared. Destroying the cache is
+			// not the responsibility of this class.
 		}
 
 		#endregion

src/NHibernate/Cache/UpdateTimestampsCache.cs

@@ -155,16 +155,12 @@ public virtual bool[] AreUpToDate(ISet<string>[] spaces, long[] timestamps)
 			return results;
 		}
 
+		// Since v5.2
+		[Obsolete("This method has no usages anymore")]
 		public virtual void Destroy()
 		{
-			try
-			{
-				_updateTimestamps.Destroy();
-			}
-			catch (Exception e)
-			{
-				log.Warn(e, "could not destroy UpdateTimestamps cache");
-			}
+			// The cache is externally provided and may be shared. Destroying the cache is
+			// not the responsibility of this class.
 		}
 
 		private bool IsOutdated(long? lastUpdate, long timestamp)

src/NHibernate/Impl/SessionFactoryImpl.cs

@@ -368,7 +368,7 @@ public SessionFactoryImpl(Configuration cfg, IMapping mapping, Settings settings
 			if (settings.IsQueryCacheEnabled)
 			{
 				var updateTimestampsCacheName = typeof(UpdateTimestampsCache).Name;
-				updateTimestampsCache = new UpdateTimestampsCache(BuildCache(updateTimestampsCacheName));
+				updateTimestampsCache = new UpdateTimestampsCache(GetCache(updateTimestampsCacheName));
 				var queryCacheName = typeof(StandardQueryCache).FullName;
 				queryCache = BuildQueryCache(queryCacheName);
 				queryCaches = new ConcurrentDictionary<string, Lazy<IQueryCache>>();
@@ -415,7 +415,7 @@ private IQueryCache BuildQueryCache(string queryCacheName)
 				settings.QueryCacheFactory.GetQueryCache(
 					updateTimestampsCache,
 					properties,
-					BuildCache(queryCacheName))
+					GetCache(queryCacheName))
 				// 6.0 TODO: remove the coalesce once IQueryCacheFactory todos are done
 #pragma warning disable 618
 				?? settings.QueryCacheFactory.GetQueryCache(
@@ -432,14 +432,14 @@ private ICacheConcurrencyStrategy GetCacheConcurrencyStrategy(
 			bool isMutable,
 			Dictionary<Tuple<string, string>, ICacheConcurrencyStrategy> caches)
 		{
-			var cacheKey = new Tuple<string, string>(cacheRegion, strategy);
 			if (strategy == null || !settings.IsSecondLevelCacheEnabled)
 				return null;
 
+			var cacheKey = new Tuple<string, string>(cacheRegion, strategy);
 			if (caches.TryGetValue(cacheKey, out var cache)) 
 				return cache;
-			
-			cache = CacheFactory.CreateCache(strategy, BuildCache(cacheRegion));
+
+			cache = CacheFactory.CreateCache(strategy, GetCache(cacheRegion));
 			caches.Add(cacheKey, cache);
 			if (isMutable && strategy == CacheFactory.ReadOnly)
 				log.Warn("read-only cache configured for mutable: {0}", name);
@@ -878,8 +878,11 @@ public void Close()
 				{
 					cache.Value.Destroy();
 				}
+			}
 
-				updateTimestampsCache.Destroy();
+			foreach (var cache in _allCacheRegions.Values)
+			{
+				cache.Destroy();
 			}
 
 			settings.CacheProvider.Stop();
@@ -1083,7 +1086,7 @@ public ICache GetSecondLevelCacheRegion(string regionName)
 			return result;
 		}
 
-		private CacheBase BuildCache(string cacheRegion)
+		private CacheBase GetCache(string cacheRegion)
 		{
 			// If run concurrently for the same region and type, this may built many caches for the same region and type.
 			// Currently only GetQueryCache may be run concurrently, and its implementation prevents