From 06510f85fda0be564233a01a6acf94406db5029e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Sun, 21 Aug 2022 19:44:58 +0200 Subject: [PATCH] Guards against use of a disposed session factory Help diagnose #3026 complains --- .../Async/SessionBuilder/Fixture.cs | 17 ++++++++++ src/NHibernate.Test/SessionBuilder/Fixture.cs | 31 +++++++++++++++++++ .../Async/Impl/SessionFactoryImpl.cs | 2 ++ src/NHibernate/Impl/SessionFactoryImpl.cs | 31 +++++++++++++++++-- 4 files changed, 79 insertions(+), 2 deletions(-) diff --git a/src/NHibernate.Test/Async/SessionBuilder/Fixture.cs b/src/NHibernate.Test/Async/SessionBuilder/Fixture.cs index 0cd255eae26..7f152be9db1 100644 --- a/src/NHibernate.Test/Async/SessionBuilder/Fixture.cs +++ b/src/NHibernate.Test/Async/SessionBuilder/Fixture.cs @@ -227,5 +227,22 @@ private void CanSetOnStateless( Assert.AreEqual(sb, fsb, $"{sbType}: Unexpected fluent return after call with {value}"); } } + + [Test] + public void ThrowWhenUsingSessionFromDisposedFactoryAsync() + { + using (var session = Sfi.OpenSession()) + { + try + { + Sfi.Dispose(); + Assert.That(() => session.GetAsync(Guid.Empty), Throws.InstanceOf(typeof(ObjectDisposedException))); + } + finally + { + RebuildSessionFactory(); + } + } + } } } diff --git a/src/NHibernate.Test/SessionBuilder/Fixture.cs b/src/NHibernate.Test/SessionBuilder/Fixture.cs index dcb7fdc393d..714c5ddfe76 100644 --- a/src/NHibernate.Test/SessionBuilder/Fixture.cs +++ b/src/NHibernate.Test/SessionBuilder/Fixture.cs @@ -298,5 +298,36 @@ private void CanSetOnStateless( Assert.AreEqual(sb, fsb, $"{sbType}: Unexpected fluent return after call with {value}"); } } + + [Test] + public void ThrowWhenOpeningFromDisposedFactory() + { + try + { + Sfi.Dispose(); + Assert.That(Sfi.OpenSession, Throws.InstanceOf(typeof(ObjectDisposedException))); + } + finally + { + RebuildSessionFactory(); + } + } + + [Test] + public void ThrowWhenUsingSessionFromDisposedFactory() + { + using (var session = Sfi.OpenSession()) + { + try + { + Sfi.Dispose(); + Assert.That(() => session.Get(Guid.Empty), Throws.InstanceOf(typeof(ObjectDisposedException))); + } + finally + { + RebuildSessionFactory(); + } + } + } } } diff --git a/src/NHibernate/Async/Impl/SessionFactoryImpl.cs b/src/NHibernate/Async/Impl/SessionFactoryImpl.cs index 50d4a4d6a11..a5984f3ada8 100644 --- a/src/NHibernate/Async/Impl/SessionFactoryImpl.cs +++ b/src/NHibernate/Async/Impl/SessionFactoryImpl.cs @@ -353,6 +353,7 @@ async Task InternalEvictCollectionAsync() // NH Different implementation if (queryCache != null) { + CheckNotClosed(); await (queryCache.ClearAsync(cancellationToken)).ConfigureAwait(false); if (queryCaches.Count == 0) { @@ -377,6 +378,7 @@ async Task InternalEvictCollectionAsync() { if (settings.IsQueryCacheEnabled) { + CheckNotClosed(); if (queryCaches.TryGetValue(cacheRegion, out var currentQueryCache)) { return currentQueryCache.Value.ClearAsync(cancellationToken); diff --git a/src/NHibernate/Impl/SessionFactoryImpl.cs b/src/NHibernate/Impl/SessionFactoryImpl.cs index 6892792c285..b25b45410e6 100644 --- a/src/NHibernate/Impl/SessionFactoryImpl.cs +++ b/src/NHibernate/Impl/SessionFactoryImpl.cs @@ -469,7 +469,11 @@ private ICacheConcurrencyStrategy GetCacheConcurrencyStrategy( public EventListeners EventListeners { - get { return eventListeners; } + get + { + CheckNotClosed(); + return eventListeners; + } } #region IObjectReference Members @@ -513,6 +517,7 @@ public object GetRealObject(StreamingContext context) public ISessionBuilder WithOptions() { + CheckNotClosed(); return new SessionBuilderImpl(this); } @@ -563,6 +568,7 @@ public ISession OpenSession(DbConnection connection, bool flushBeforeCompletionE public IStatelessSessionBuilder WithStatelessOptions() { + CheckNotClosed(); return new StatelessSessionBuilderImpl(this); } @@ -580,6 +586,7 @@ public IStatelessSession OpenStatelessSession(DbConnection connection) public IEntityPersister GetEntityPersister(string entityName) { + CheckNotClosed(); IEntityPersister value; if (entityPersisters.TryGetValue(entityName, out value) == false) throw new MappingException("No persister for: " + entityName); @@ -588,6 +595,7 @@ public IEntityPersister GetEntityPersister(string entityName) public IEntityPersister TryGetEntityPersister(string entityName) { + CheckNotClosed(); IEntityPersister result; entityPersisters.TryGetValue(entityName, out result); return result; @@ -595,6 +603,7 @@ public IEntityPersister TryGetEntityPersister(string entityName) public ICollectionPersister GetCollectionPersister(string role) { + CheckNotClosed(); ICollectionPersister value; if (collectionPersisters.TryGetValue(role, out value) == false) throw new MappingException("Unknown collection role: " + role); @@ -863,6 +872,14 @@ public void Dispose() Close(); } + private void CheckNotClosed() + { + if (isClosed) + { + throw new ObjectDisposedException($"Session factory {Name} with id {Uuid}"); + } + } + /// /// Closes the session factory, releasing all held resources. /// @@ -1105,6 +1122,7 @@ public UpdateTimestampsCache UpdateTimestampsCache public IDictionary GetAllSecondLevelCacheRegions() #pragma warning restore 618 { + CheckNotClosed(); return _allCacheRegions // ToArray creates a moment in time snapshot @@ -1119,6 +1137,7 @@ public IDictionary GetAllSecondLevelCacheRegions() public ICache GetSecondLevelCacheRegion(string regionName) #pragma warning restore 618 { + CheckNotClosed(); _allCacheRegions.TryGetValue(regionName, out var result); return result; } @@ -1145,7 +1164,12 @@ public IStatisticsImplementor StatisticsImplementor public IQueryCache QueryCache { - get { return queryCache; } + get + { + if (queryCache != null) + CheckNotClosed(); + return queryCache; + } } public IQueryCache GetQueryCache(string cacheRegion) @@ -1159,6 +1183,7 @@ public IQueryCache GetQueryCache(string cacheRegion) return null; } + CheckNotClosed(); // 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 @@ -1175,6 +1200,7 @@ public void EvictQueries() // NH Different implementation if (queryCache != null) { + CheckNotClosed(); queryCache.Clear(); if (queryCaches.Count == 0) { @@ -1193,6 +1219,7 @@ public void EvictQueries(string cacheRegion) { if (settings.IsQueryCacheEnabled) { + CheckNotClosed(); if (queryCaches.TryGetValue(cacheRegion, out var currentQueryCache)) { currentQueryCache.Value.Clear();