diff --git a/src/NHibernate.Test/Async/NHSpecificTest/NH4077/PostInsertFixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/NH4077/PostInsertFixture.cs new file mode 100644 index 00000000000..50bdc4f532a --- /dev/null +++ b/src/NHibernate.Test/Async/NHSpecificTest/NH4077/PostInsertFixture.cs @@ -0,0 +1,185 @@ +//------------------------------------------------------------------------------ +// +// 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.Linq; +using NHibernate.Cfg; +using NHibernate.Cfg.MappingSchema; +using NHibernate.Event; +using NHibernate.Mapping.ByCode; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.NH4077 +{ + using System.Threading.Tasks; + using System.Threading; + [TestFixture] + public partial class PostInsertFixtureAsync : TestCaseMappingByCode + { + [Test] + public async Task AutoflushInPostInsertListener_CausesDuplicateInserts_WithPrimaryKeyViolationsAsync() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + // using FlushMode.Commit prevents the issue; using the default FlushMode.Auto breaks. + //session.FlushMode = FlushMode.Commit; + await (session.SaveAsync(new Entity { Code = "one" })); + await (session.SaveAsync(new Entity { Code = "two" })); + + // committing the transaction causes a primary key violation by saving the entities multiple times + await (transaction.CommitAsync()); + await (session.FlushAsync()); + } + } + + [Test] + public async Task Autoflush_MayTriggerAdditionalAutoFlushFromEventsAsync() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + // using FlushMode.Commit prevents the issue; using the default FlushMode.Auto breaks. + //session.FlushMode = FlushMode.Commit; + await (session.SaveAsync(new Entity { Code = "one" })); + await (session.SaveAsync(new Entity { Code = "two" })); + + // Querying the entity triggers an auto-flush + var count = await (session.CreateQuery("select count(o) from Entity o").UniqueResultAsync()); + Assert.That(count, Is.GreaterThan(0)); + await (transaction.CommitAsync()); + await (session.FlushAsync()); + } + } + + protected override HbmMapping GetMappings() + { + var mapper = new ModelMapper(); + mapper.Class(rc => + { + rc.Id(x => x.Id, m => m.Generator(Generators.GuidComb)); + rc.Property(x => x.Code); + }); + + return mapper.CompileMappingForAllExplicitlyAddedEntities(); + } + + protected override void Configure(Configuration configuration) + { + base.Configure(configuration); + var existingListeners = (configuration.EventListeners.PostInsertEventListeners ?? new IPostInsertEventListener[0]).ToList(); + // this evil listener uses the session to perform a few queries and causes an auto-flush to happen + existingListeners.Add(new CausesAutoflushListener()); + configuration.EventListeners.PostInsertEventListeners = existingListeners.ToArray(); + } + + protected override void OnTearDown() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + session.Delete("from Entity"); + + session.Flush(); + transaction.Commit(); + } + } + + private sealed partial class CausesAutoflushListener : IPostInsertEventListener + { + private bool _currentlyLogging; + + public async Task OnPostInsertAsync(PostInsertEvent @event, CancellationToken cancellationToken) + { + if (!(@event.Entity is Entity)) + return; + // This guard is necessary to avoid multiple inserts of the original objects. + // Commenting this out is likely to cause one PK violation per run, which seems to be capped to at most 10 attempts. + // With the guard, only one PK violation is reported. + if (_currentlyLogging) + return; + + try + { + _currentlyLogging = true; + var session = @event.Session; + // this causes an Autoflush + long count = await (session.CreateQuery("select count(o) from Entity o").UniqueResultAsync(cancellationToken)); + Console.WriteLine("Total entity count: {0}", count); + } + finally + { + _currentlyLogging = false; + } + } + + public void OnPostInsert(PostInsertEvent @event) + { + if (!(@event.Entity is Entity)) + return; + // This guard is necessary to avoid multiple inserts of the original objects. + // Commenting this out is likely to cause one PK violation per run, which seems to be capped to at most 10 attempts. + // With the guard, only one PK violation is reported. + if (_currentlyLogging) + return; + + try + { + _currentlyLogging = true; + var session = @event.Session; + // this causes an Autoflush + long count = session.CreateQuery("select count(o) from Entity o").UniqueResult(); + Console.WriteLine("Total entity count: {0}", count); + } + finally + { + _currentlyLogging = false; + } + } + } + } + /// + /// Contains generated async methods + /// + public partial class PostInsertFixture : TestCaseMappingByCode + { + + /// + /// Contains generated async methods + /// + private sealed partial class CausesAutoflushListener : IPostInsertEventListener + { + + public async Task OnPostInsertAsync(PostInsertEvent @event, CancellationToken cancellationToken) + { + if (!(@event.Entity is Entity)) + return; + // This guard is necessary to avoid multiple inserts of the original objects. + // Commenting this out is likely to cause one PK violation per run, which seems to be capped to at most 10 attempts. + // With the guard, only one PK violation is reported. + if (_currentlyLogging) + return; + + try + { + _currentlyLogging = true; + var session = @event.Session; + // this causes an Autoflush + long count = await (session.CreateQuery("select count(o) from Entity o").UniqueResultAsync(cancellationToken)); + Console.WriteLine("Total entity count: {0}", count); + } + finally + { + _currentlyLogging = false; + } + } + } + } +} diff --git a/src/NHibernate.Test/Async/NHSpecificTest/NH4077/PostUpdateFixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/NH4077/PostUpdateFixture.cs new file mode 100644 index 00000000000..a01c2556c64 --- /dev/null +++ b/src/NHibernate.Test/Async/NHSpecificTest/NH4077/PostUpdateFixture.cs @@ -0,0 +1,187 @@ +//------------------------------------------------------------------------------ +// +// 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.Linq; +using NHibernate.Cfg; +using NHibernate.Cfg.MappingSchema; +using NHibernate.Criterion; +using NHibernate.Event; +using NHibernate.Mapping.ByCode; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.NH4077 +{ + using System.Threading.Tasks; + using System.Threading; + [TestFixture] + public partial class PostUpdateFixtureAsync : TestCaseMappingByCode + { + [Test] + public async Task AutoflushInPostUpdateListener_CausesArgumentOutOfRangeException_in_ActionQueueExecuteActionsAsync() + { + // load a few (more than one) entities and process them. we let NHibernate figure out if they need saving or not. + Entity entityOne; + Entity entityTwo; + using (var session = OpenSession()) + { + entityOne = (await (session.CreateCriteria().Add(Restrictions.Eq(nameof(Entity.Code), "one")).ListAsync())).First(); + entityTwo = (await (session.CreateCriteria().Add(Restrictions.Eq(nameof(Entity.Code), "two")).ListAsync())).First(); + } + + // processing omitted (not necessary to illustrate the problem) + + // resave them, but all-or-nothing inside a transaction + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + // using FlushMode.Commit prevents the issue; using the default FlushMode.Auto breaks. + //session.FlushMode = FlushMode.Commit; + await (session.SaveOrUpdateAsync(entityOne)); + await (session.SaveOrUpdateAsync(entityTwo)); + + // committing the transaction causes an ArgumentOutOfRange exception inside ActionQueue.ExecuteActions + await (transaction.CommitAsync()); + await (session.FlushAsync()); + } + } + + protected override HbmMapping GetMappings() + { + var mapper = new ModelMapper(); + mapper.Class(rc => + { + rc.Id(x => x.Id, m => m.Generator(Generators.GuidComb)); + rc.Property(x => x.Code); + }); + + return mapper.CompileMappingForAllExplicitlyAddedEntities(); + } + + protected override void Configure(Configuration configuration) + { + base.Configure(configuration); + var existingListeners = (configuration.EventListeners.PostUpdateEventListeners ?? new IPostUpdateEventListener[0]).ToList(); + // this evil listener uses the session to perform a few queries and causes an auto-flush to happen + existingListeners.Add(new CausesAutoflushListener()); + configuration.EventListeners.PostUpdateEventListeners = existingListeners.ToArray(); + } + + protected override void OnTearDown() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + session.Delete("from Entity"); + + session.Flush(); + transaction.Commit(); + } + } + + protected override void OnSetUp() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + // objects must exist before doing the processing; the issue does not occur during + session.Save(new Entity { Code = "one" }); + session.Save(new Entity { Code = "two" }); + + session.Flush(); + transaction.Commit(); + } + } + + private sealed partial class CausesAutoflushListener : IPostUpdateEventListener + { + private bool _currentlyLogging; + + public async Task OnPostUpdateAsync(PostUpdateEvent @event, CancellationToken cancellationToken) + { + if (!(@event.Entity is Entity)) + return; + // this guard is necessary to avoid a StackOverflowException due to the Query below triggering this event again. + if (_currentlyLogging) + return; + + try + { + _currentlyLogging = true; + var session = @event.Session; + // this causes an Autoflush + long count = await (session.CreateQuery("select count(o) from Entity o").UniqueResultAsync(cancellationToken)); + Console.WriteLine("Total entity count: {0}", count); + } + finally + { + _currentlyLogging = false; + } + } + + public void OnPostUpdate(PostUpdateEvent @event) + { + if (!(@event.Entity is Entity)) + return; + // this guard is necessary to avoid a StackOverflowException due to the Query below triggering this event again. + if (_currentlyLogging) + return; + + try + { + _currentlyLogging = true; + var session = @event.Session; + // this causes an Autoflush + long count = session.CreateQuery("select count(o) from Entity o").UniqueResult(); + Console.WriteLine("Total entity count: {0}", count); + } + finally + { + _currentlyLogging = false; + } + } + } + } + /// + /// Contains generated async methods + /// + public partial class PostUpdateFixture : TestCaseMappingByCode + { + + /// + /// Contains generated async methods + /// + private sealed partial class CausesAutoflushListener : IPostUpdateEventListener + { + + public async Task OnPostUpdateAsync(PostUpdateEvent @event, CancellationToken cancellationToken) + { + if (!(@event.Entity is Entity)) + return; + // this guard is necessary to avoid a StackOverflowException due to the Query below triggering this event again. + if (_currentlyLogging) + return; + + try + { + _currentlyLogging = true; + var session = @event.Session; + // this causes an Autoflush + long count = await (session.CreateQuery("select count(o) from Entity o").UniqueResultAsync(cancellationToken)); + Console.WriteLine("Total entity count: {0}", count); + } + finally + { + _currentlyLogging = false; + } + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/NH4077/Model.cs b/src/NHibernate.Test/NHSpecificTest/NH4077/Model.cs new file mode 100644 index 00000000000..5cbf18b27f4 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH4077/Model.cs @@ -0,0 +1,10 @@ +using System; + +namespace NHibernate.Test.NHSpecificTest.NH4077 +{ + public class Entity + { + public virtual Guid Id { get; set; } + public virtual string Code { get; set; } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/NH4077/PostInsertFixture.cs b/src/NHibernate.Test/NHSpecificTest/NH4077/PostInsertFixture.cs new file mode 100644 index 00000000000..9e02d22dba4 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH4077/PostInsertFixture.cs @@ -0,0 +1,112 @@ +using System; +using System.Linq; +using NHibernate.Cfg; +using NHibernate.Cfg.MappingSchema; +using NHibernate.Event; +using NHibernate.Mapping.ByCode; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.NH4077 +{ + [TestFixture] + public partial class PostInsertFixture : TestCaseMappingByCode + { + [Test] + public void AutoflushInPostInsertListener_CausesDuplicateInserts_WithPrimaryKeyViolations() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + // using FlushMode.Commit prevents the issue; using the default FlushMode.Auto breaks. + //session.FlushMode = FlushMode.Commit; + session.Save(new Entity { Code = "one" }); + session.Save(new Entity { Code = "two" }); + + // committing the transaction causes a primary key violation by saving the entities multiple times + transaction.Commit(); + session.Flush(); + } + } + + [Test] + public void Autoflush_MayTriggerAdditionalAutoFlushFromEvents() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + // using FlushMode.Commit prevents the issue; using the default FlushMode.Auto breaks. + //session.FlushMode = FlushMode.Commit; + session.Save(new Entity { Code = "one" }); + session.Save(new Entity { Code = "two" }); + + // Querying the entity triggers an auto-flush + var count = session.CreateQuery("select count(o) from Entity o").UniqueResult(); + Assert.That(count, Is.GreaterThan(0)); + transaction.Commit(); + session.Flush(); + } + } + + protected override HbmMapping GetMappings() + { + var mapper = new ModelMapper(); + mapper.Class(rc => + { + rc.Id(x => x.Id, m => m.Generator(Generators.GuidComb)); + rc.Property(x => x.Code); + }); + + return mapper.CompileMappingForAllExplicitlyAddedEntities(); + } + + protected override void Configure(Configuration configuration) + { + base.Configure(configuration); + var existingListeners = (configuration.EventListeners.PostInsertEventListeners ?? new IPostInsertEventListener[0]).ToList(); + // this evil listener uses the session to perform a few queries and causes an auto-flush to happen + existingListeners.Add(new CausesAutoflushListener()); + configuration.EventListeners.PostInsertEventListeners = existingListeners.ToArray(); + } + + protected override void OnTearDown() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + session.Delete("from Entity"); + + session.Flush(); + transaction.Commit(); + } + } + + private sealed partial class CausesAutoflushListener : IPostInsertEventListener + { + private bool _currentlyLogging; + + public void OnPostInsert(PostInsertEvent @event) + { + if (!(@event.Entity is Entity)) + return; + // This guard is necessary to avoid multiple inserts of the original objects. + // Commenting this out is likely to cause one PK violation per run, which seems to be capped to at most 10 attempts. + // With the guard, only one PK violation is reported. + if (_currentlyLogging) + return; + + try + { + _currentlyLogging = true; + var session = @event.Session; + // this causes an Autoflush + long count = session.CreateQuery("select count(o) from Entity o").UniqueResult(); + Console.WriteLine("Total entity count: {0}", count); + } + finally + { + _currentlyLogging = false; + } + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/NH4077/PostUpdateFixture.cs b/src/NHibernate.Test/NHSpecificTest/NH4077/PostUpdateFixture.cs new file mode 100644 index 00000000000..8e9d9932b83 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH4077/PostUpdateFixture.cs @@ -0,0 +1,118 @@ +using System; +using System.Linq; +using NHibernate.Cfg; +using NHibernate.Cfg.MappingSchema; +using NHibernate.Criterion; +using NHibernate.Event; +using NHibernate.Mapping.ByCode; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.NH4077 +{ + [TestFixture] + public partial class PostUpdateFixture : TestCaseMappingByCode + { + [Test] + public void AutoflushInPostUpdateListener_CausesArgumentOutOfRangeException_in_ActionQueueExecuteActions() + { + // load a few (more than one) entities and process them. we let NHibernate figure out if they need saving or not. + Entity entityOne; + Entity entityTwo; + using (var session = OpenSession()) + { + entityOne = session.CreateCriteria().Add(Restrictions.Eq(nameof(Entity.Code), "one")).List().First(); + entityTwo = session.CreateCriteria().Add(Restrictions.Eq(nameof(Entity.Code), "two")).List().First(); + } + + // processing omitted (not necessary to illustrate the problem) + + // resave them, but all-or-nothing inside a transaction + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + // using FlushMode.Commit prevents the issue; using the default FlushMode.Auto breaks. + //session.FlushMode = FlushMode.Commit; + session.SaveOrUpdate(entityOne); + session.SaveOrUpdate(entityTwo); + + // committing the transaction causes an ArgumentOutOfRange exception inside ActionQueue.ExecuteActions + transaction.Commit(); + session.Flush(); + } + } + + protected override HbmMapping GetMappings() + { + var mapper = new ModelMapper(); + mapper.Class(rc => + { + rc.Id(x => x.Id, m => m.Generator(Generators.GuidComb)); + rc.Property(x => x.Code); + }); + + return mapper.CompileMappingForAllExplicitlyAddedEntities(); + } + + protected override void Configure(Configuration configuration) + { + base.Configure(configuration); + var existingListeners = (configuration.EventListeners.PostUpdateEventListeners ?? new IPostUpdateEventListener[0]).ToList(); + // this evil listener uses the session to perform a few queries and causes an auto-flush to happen + existingListeners.Add(new CausesAutoflushListener()); + configuration.EventListeners.PostUpdateEventListeners = existingListeners.ToArray(); + } + + protected override void OnTearDown() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + session.Delete("from Entity"); + + session.Flush(); + transaction.Commit(); + } + } + + protected override void OnSetUp() + { + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + // objects must exist before doing the processing; the issue does not occur during + session.Save(new Entity { Code = "one" }); + session.Save(new Entity { Code = "two" }); + + session.Flush(); + transaction.Commit(); + } + } + + private sealed partial class CausesAutoflushListener : IPostUpdateEventListener + { + private bool _currentlyLogging; + + public void OnPostUpdate(PostUpdateEvent @event) + { + if (!(@event.Entity is Entity)) + return; + // this guard is necessary to avoid a StackOverflowException due to the Query below triggering this event again. + if (_currentlyLogging) + return; + + try + { + _currentlyLogging = true; + var session = @event.Session; + // this causes an Autoflush + long count = session.CreateQuery("select count(o) from Entity o").UniqueResult(); + Console.WriteLine("Total entity count: {0}", count); + } + finally + { + _currentlyLogging = false; + } + } + } + } +} diff --git a/src/NHibernate/Async/Engine/ActionQueue.cs b/src/NHibernate/Async/Engine/ActionQueue.cs index 2a0b05e4d86..579cd7ac2a3 100644 --- a/src/NHibernate/Async/Engine/ActionQueue.cs +++ b/src/NHibernate/Async/Engine/ActionQueue.cs @@ -40,9 +40,12 @@ public Task AddActionAsync(BulkOperationCleanupAction cleanupAction, Cancellatio private async Task ExecuteActionsAsync(IList list, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - int size = list.Count; - for (int i = 0; i < size; i++) - await (ExecuteAsync((IExecutable)list[i], cancellationToken)).ConfigureAwait(false); + // Actions may raise events to which user code can react and cause changes to action list. + // It will then fail here due to list being modified. (Some previous code was dodging the + // trouble with a for loop which was not failing provided the list was not getting smaller. + // But then it was clearing it without having executed added actions (if any), ...) + foreach (IExecutable executable in list) + await (ExecuteAsync(executable, cancellationToken)).ConfigureAwait(false); list.Clear(); await (session.Batcher.ExecuteBatchAsync(cancellationToken)).ConfigureAwait(false); diff --git a/src/NHibernate/Async/Event/Default/DefaultAutoFlushEventListener.cs b/src/NHibernate/Async/Event/Default/DefaultAutoFlushEventListener.cs index 8bf233fcd54..864e00e3487 100644 --- a/src/NHibernate/Async/Event/Default/DefaultAutoFlushEventListener.cs +++ b/src/NHibernate/Async/Event/Default/DefaultAutoFlushEventListener.cs @@ -36,34 +36,37 @@ public virtual async Task OnAutoFlushAsync(AutoFlushEvent @event, CancellationTo if (FlushMightBeNeeded(source)) { - int oldSize = source.ActionQueue.CollectionRemovalsCount; + using (source.SuspendAutoFlush()) + { + int oldSize = source.ActionQueue.CollectionRemovalsCount; - await (FlushEverythingToExecutionsAsync(@event, cancellationToken)).ConfigureAwait(false); + await (FlushEverythingToExecutionsAsync(@event, cancellationToken)).ConfigureAwait(false); - if (FlushIsReallyNeeded(@event, source)) - { - if (log.IsDebugEnabled) - log.Debug("Need to execute flush"); + if (FlushIsReallyNeeded(@event, source)) + { + if (log.IsDebugEnabled) + log.Debug("Need to execute flush"); - await (PerformExecutionsAsync(source, cancellationToken)).ConfigureAwait(false); - PostFlush(source); - // note: performExecutions() clears all collectionXxxxtion - // collections (the collection actions) in the session + await (PerformExecutionsAsync(source, cancellationToken)).ConfigureAwait(false); + PostFlush(source); + // note: performExecutions() clears all collectionXxxxtion + // collections (the collection actions) in the session - if (source.Factory.Statistics.IsStatisticsEnabled) + if (source.Factory.Statistics.IsStatisticsEnabled) + { + source.Factory.StatisticsImplementor.Flush(); + } + } + else { - source.Factory.StatisticsImplementor.Flush(); + + if (log.IsDebugEnabled) + log.Debug("Dont need to execute flush"); + source.ActionQueue.ClearFromFlushNeededCheck(oldSize); } - } - else - { - if (log.IsDebugEnabled) - log.Debug("Dont need to execute flush"); - source.ActionQueue.ClearFromFlushNeededCheck(oldSize); + @event.FlushRequired = FlushIsReallyNeeded(@event, source); } - - @event.FlushRequired = FlushIsReallyNeeded(@event, source); } } diff --git a/src/NHibernate/Async/Impl/SessionImpl.cs b/src/NHibernate/Async/Impl/SessionImpl.cs index fe60e80f398..441e6352993 100644 --- a/src/NHibernate/Async/Impl/SessionImpl.cs +++ b/src/NHibernate/Async/Impl/SessionImpl.cs @@ -233,25 +233,26 @@ public override async Task ListAsync(IQueryExpression queryExpression, QueryPara await (AutoFlushIfRequiredAsync(plan.QuerySpaces, cancellationToken)).ConfigureAwait(false); bool success = false; - dontFlushFromFind++; //stops flush being called multiple times if this method is recursively called - try - { - await (plan.PerformListAsync(queryParameters, this, results, cancellationToken)).ConfigureAwait(false); - success = true; - } - catch (HibernateException) - { - // Do not call Convert on HibernateExceptions - throw; - } - catch (Exception e) - { - throw Convert(e, "Could not execute query"); - } - finally + using (SuspendAutoFlush()) //stops flush being called multiple times if this method is recursively called { - dontFlushFromFind--; - await (AfterOperationAsync(success, cancellationToken)).ConfigureAwait(false); + try + { + await (plan.PerformListAsync(queryParameters, this, results, cancellationToken)).ConfigureAwait(false); + success = true; + } + catch (HibernateException) + { + // Do not call Convert on HibernateExceptions + throw; + } + catch (Exception e) + { + throw Convert(e, "Could not execute query"); + } + finally + { + await (AfterOperationAsync(success, cancellationToken)).ConfigureAwait(false); + } } } } @@ -277,15 +278,10 @@ public override async Task> EnumerableAsync(IQueryExpression q var plan = GetHQLQueryPlan(queryExpression, true); await (AutoFlushIfRequiredAsync(plan.QuerySpaces, cancellationToken)).ConfigureAwait(false); - dontFlushFromFind++; //stops flush being called multiple times if this method is recursively called - try + using (SuspendAutoFlush()) //stops flush being called multiple times if this method is recursively called { return await (plan.PerformIterateAsync(queryParameters, this, cancellationToken)).ConfigureAwait(false); } - finally - { - dontFlushFromFind--; - } } } @@ -299,15 +295,10 @@ public override async Task EnumerableAsync(IQueryExpression queryEx var plan = GetHQLQueryPlan(queryExpression, true); await (AutoFlushIfRequiredAsync(plan.QuerySpaces, cancellationToken)).ConfigureAwait(false); - dontFlushFromFind++; //stops flush being called multiple times if this method is recursively called - try + using (SuspendAutoFlush()) //stops flush being called multiple times if this method is recursively called { return await (plan.PerformIterateAsync(queryParameters, this, cancellationToken)).ConfigureAwait(false); } - finally - { - dontFlushFromFind--; - } } } @@ -961,10 +952,13 @@ public override async Task InternalLoadAsync(string entityName, object i { throw new HibernateException("Flush during cascade is dangerous"); } - IFlushEventListener[] flushEventListener = listeners.FlushEventListeners; - for (int i = 0; i < flushEventListener.Length; i++) + using (SuspendAutoFlush()) { - await (flushEventListener[i].OnFlushAsync(new FlushEvent(this), cancellationToken)).ConfigureAwait(false); + IFlushEventListener[] flushEventListener = listeners.FlushEventListeners; + for (int i = 0; i < flushEventListener.Length; i++) + { + await (flushEventListener[i].OnFlushAsync(new FlushEvent(this), cancellationToken)).ConfigureAwait(false); + } } } } @@ -1024,25 +1018,26 @@ private async Task FilterAsync(object collection, string filter, QueryParameters FilterQueryPlan plan = await (GetFilterQueryPlanAsync(collection, filter, queryParameters, false, cancellationToken)).ConfigureAwait(false); bool success = false; - dontFlushFromFind++; //stops flush being called multiple times if this method is recursively called - try - { - await (plan.PerformListAsync(queryParameters, this, results, cancellationToken)).ConfigureAwait(false); - success = true; - } - catch (HibernateException) - { - // Do not call Convert on HibernateExceptions - throw; - } - catch (Exception e) - { - throw Convert(e, "could not execute query"); - } - finally + using (SuspendAutoFlush()) //stops flush being called multiple times if this method is recursively called { - dontFlushFromFind--; - await (AfterOperationAsync(success, cancellationToken)).ConfigureAwait(false); + try + { + await (plan.PerformListAsync(queryParameters, this, results, cancellationToken)).ConfigureAwait(false); + success = true; + } + catch (HibernateException) + { + // Do not call Convert on HibernateExceptions + throw; + } + catch (Exception e) + { + throw Convert(e, "could not execute query"); + } + finally + { + await (AfterOperationAsync(success, cancellationToken)).ConfigureAwait(false); + } } } } @@ -1119,30 +1114,30 @@ public override async Task ListAsync(CriteriaImpl criteria, IList results, Cance await (AutoFlushIfRequiredAsync(spaces, cancellationToken)).ConfigureAwait(false); - dontFlushFromFind++; - bool success = false; - try + using (SuspendAutoFlush()) { - for (int i = size - 1; i >= 0; i--) + try { - ArrayHelper.AddAll(results, await (loaders[i].ListAsync(this, cancellationToken)).ConfigureAwait(false)); + for (int i = size - 1; i >= 0; i--) + { + ArrayHelper.AddAll(results, await (loaders[i].ListAsync(this, cancellationToken)).ConfigureAwait(false)); + } + success = true; + } + catch (HibernateException) + { + // Do not call Convert on HibernateExceptions + throw; + } + catch (Exception sqle) + { + throw Convert(sqle, "Unable to perform find"); + } + finally + { + await (AfterOperationAsync(success, cancellationToken)).ConfigureAwait(false); } - success = true; - } - catch (HibernateException) - { - // Do not call Convert on HibernateExceptions - throw; - } - catch (Exception sqle) - { - throw Convert(sqle, "Unable to perform find"); - } - finally - { - dontFlushFromFind--; - await (AfterOperationAsync(success, cancellationToken)).ConfigureAwait(false); } } } @@ -1173,16 +1168,17 @@ public override async Task ListCustomQueryAsync(ICustomQuery customQuery, QueryP await (AutoFlushIfRequiredAsync(loader.QuerySpaces, cancellationToken)).ConfigureAwait(false); bool success = false; - dontFlushFromFind++; - try - { - ArrayHelper.AddAll(results, await (loader.ListAsync(this, queryParameters, cancellationToken)).ConfigureAwait(false)); - success = true; - } - finally + using (SuspendAutoFlush()) { - dontFlushFromFind--; - await (AfterOperationAsync(success, cancellationToken)).ConfigureAwait(false); + try + { + ArrayHelper.AddAll(results, await (loader.ListAsync(this, queryParameters, cancellationToken)).ConfigureAwait(false)); + success = true; + } + finally + { + await (AfterOperationAsync(success, cancellationToken)).ConfigureAwait(false); + } } } } diff --git a/src/NHibernate/Engine/ActionQueue.cs b/src/NHibernate/Engine/ActionQueue.cs index 7dc27963d05..1d2b7ef32d4 100644 --- a/src/NHibernate/Engine/ActionQueue.cs +++ b/src/NHibernate/Engine/ActionQueue.cs @@ -121,9 +121,12 @@ public void RegisterProcess(BeforeTransactionCompletionProcessDelegate process) private void ExecuteActions(IList list) { - int size = list.Count; - for (int i = 0; i < size; i++) - Execute((IExecutable)list[i]); + // Actions may raise events to which user code can react and cause changes to action list. + // It will then fail here due to list being modified. (Some previous code was dodging the + // trouble with a for loop which was not failing provided the list was not getting smaller. + // But then it was clearing it without having executed added actions (if any), ...) + foreach (IExecutable executable in list) + Execute(executable); list.Clear(); session.Batcher.ExecuteBatch(); diff --git a/src/NHibernate/Engine/ISessionImplementor.cs b/src/NHibernate/Engine/ISessionImplementor.cs index 6f86b900231..01d117c01a7 100644 --- a/src/NHibernate/Engine/ISessionImplementor.cs +++ b/src/NHibernate/Engine/ISessionImplementor.cs @@ -225,8 +225,6 @@ public partial interface ISessionImplementor /// Retrieves the configured event listeners from this event source. EventListeners Listeners { get; } - int DontFlushFromFind { get; } - ConnectionManager ConnectionManager { get; } bool IsEventSource { get; } diff --git a/src/NHibernate/Event/Default/DefaultAutoFlushEventListener.cs b/src/NHibernate/Event/Default/DefaultAutoFlushEventListener.cs index 9eca5e67ea8..3133c7ec3ee 100644 --- a/src/NHibernate/Event/Default/DefaultAutoFlushEventListener.cs +++ b/src/NHibernate/Event/Default/DefaultAutoFlushEventListener.cs @@ -25,34 +25,37 @@ public virtual void OnAutoFlush(AutoFlushEvent @event) if (FlushMightBeNeeded(source)) { - int oldSize = source.ActionQueue.CollectionRemovalsCount; + using (source.SuspendAutoFlush()) + { + int oldSize = source.ActionQueue.CollectionRemovalsCount; - FlushEverythingToExecutions(@event); + FlushEverythingToExecutions(@event); - if (FlushIsReallyNeeded(@event, source)) - { - if (log.IsDebugEnabled) - log.Debug("Need to execute flush"); + if (FlushIsReallyNeeded(@event, source)) + { + if (log.IsDebugEnabled) + log.Debug("Need to execute flush"); - PerformExecutions(source); - PostFlush(source); - // note: performExecutions() clears all collectionXxxxtion - // collections (the collection actions) in the session + PerformExecutions(source); + PostFlush(source); + // note: performExecutions() clears all collectionXxxxtion + // collections (the collection actions) in the session - if (source.Factory.Statistics.IsStatisticsEnabled) + if (source.Factory.Statistics.IsStatisticsEnabled) + { + source.Factory.StatisticsImplementor.Flush(); + } + } + else { - source.Factory.StatisticsImplementor.Flush(); + + if (log.IsDebugEnabled) + log.Debug("Dont need to execute flush"); + source.ActionQueue.ClearFromFlushNeededCheck(oldSize); } - } - else - { - if (log.IsDebugEnabled) - log.Debug("Dont need to execute flush"); - source.ActionQueue.ClearFromFlushNeededCheck(oldSize); + @event.FlushRequired = FlushIsReallyNeeded(@event, source); } - - @event.FlushRequired = FlushIsReallyNeeded(@event, source); } } @@ -63,11 +66,11 @@ private bool FlushIsReallyNeeded(AutoFlushEvent @event, IEventSource source) return source.ActionQueue.AreTablesToBeUpdated(@event.QuerySpaces) || ((ISessionImplementor)source).FlushMode == FlushMode.Always; } - private bool FlushMightBeNeeded(ISessionImplementor source) + private bool FlushMightBeNeeded(IEventSource source) { return - !(source.FlushMode < FlushMode.Auto) - && source.DontFlushFromFind == 0 + !(((ISessionImplementor) source).FlushMode < FlushMode.Auto) + && !source.AutoFlushSuspended && ((source.PersistenceContext.EntityEntries.Count > 0) || (source.PersistenceContext.CollectionEntries.Count > 0)); } } diff --git a/src/NHibernate/Event/IEventSource.cs b/src/NHibernate/Event/IEventSource.cs index 054c68a8b86..55150a18181 100644 --- a/src/NHibernate/Event/IEventSource.cs +++ b/src/NHibernate/Event/IEventSource.cs @@ -11,6 +11,11 @@ public partial interface IEventSource : ISessionImplementor, ISession /// Get the ActionQueue for this session ActionQueue ActionQueue { get;} + /// + /// Is auto-flush suspended? + /// + bool AutoFlushSuspended { get; } + /// /// Instantiate an entity instance, using either an interceptor, /// or the given persister @@ -34,5 +39,12 @@ public partial interface IEventSource : ISessionImplementor, ISession /// Cascade delete an entity instance void Delete(string entityName, object child, bool isCascadeDeleteEnabled, ISet transientEntities); + + /// + /// Suspend auto-flushing, yielding a disposable to dispose when auto flush should be restored. Supports + /// being called multiple times. + /// + /// A disposable to dispose when auto flush should be restored. + IDisposable SuspendAutoFlush(); } } diff --git a/src/NHibernate/Impl/AbstractSessionImpl.cs b/src/NHibernate/Impl/AbstractSessionImpl.cs index 4c686aecdab..b0a70666a23 100644 --- a/src/NHibernate/Impl/AbstractSessionImpl.cs +++ b/src/NHibernate/Impl/AbstractSessionImpl.cs @@ -208,7 +208,6 @@ public virtual IQuery GetNamedSQLQuery(string name) public abstract IQueryTranslator[] GetQueries(IQueryExpression query, bool scalar); public abstract EventListeners Listeners { get; } - public abstract int DontFlushFromFind { get; } public abstract ConnectionManager ConnectionManager { get; } public abstract bool IsEventSource { get; } public abstract object GetEntityUsingInterceptor(EntityKey key); diff --git a/src/NHibernate/Impl/SessionImpl.cs b/src/NHibernate/Impl/SessionImpl.cs index 7e33ce98d5d..2511e356cdf 100644 --- a/src/NHibernate/Impl/SessionImpl.cs +++ b/src/NHibernate/Impl/SessionImpl.cs @@ -53,10 +53,10 @@ public sealed partial class SessionImpl : AbstractSessionImpl, IEventSource, ISe [NonSerialized] private readonly ActionQueue actionQueue; - private readonly ConnectionManager connectionManager; - [NonSerialized] - private int dontFlushFromFind; + private int _suspendAutoFlushCount; + + private readonly ConnectionManager connectionManager; [NonSerialized] private readonly IDictionary enabledFilters = new Dictionary(); @@ -536,25 +536,26 @@ public override void List(IQueryExpression queryExpression, QueryParameters quer AutoFlushIfRequired(plan.QuerySpaces); bool success = false; - dontFlushFromFind++; //stops flush being called multiple times if this method is recursively called - try - { - plan.PerformList(queryParameters, this, results); - success = true; - } - catch (HibernateException) - { - // Do not call Convert on HibernateExceptions - throw; - } - catch (Exception e) - { - throw Convert(e, "Could not execute query"); - } - finally + using (SuspendAutoFlush()) //stops flush being called multiple times if this method is recursively called { - dontFlushFromFind--; - AfterOperation(success); + try + { + plan.PerformList(queryParameters, this, results); + success = true; + } + catch (HibernateException) + { + // Do not call Convert on HibernateExceptions + throw; + } + catch (Exception e) + { + throw Convert(e, "Could not execute query"); + } + finally + { + AfterOperation(success); + } } } } @@ -578,15 +579,10 @@ public override IEnumerable Enumerable(IQueryExpression queryExpression, Q var plan = GetHQLQueryPlan(queryExpression, true); AutoFlushIfRequired(plan.QuerySpaces); - dontFlushFromFind++; //stops flush being called multiple times if this method is recursively called - try + using (SuspendAutoFlush()) //stops flush being called multiple times if this method is recursively called { return plan.PerformIterate(queryParameters, this); } - finally - { - dontFlushFromFind--; - } } } @@ -599,15 +595,10 @@ public override IEnumerable Enumerable(IQueryExpression queryExpression, QueryPa var plan = GetHQLQueryPlan(queryExpression, true); AutoFlushIfRequired(plan.QuerySpaces); - dontFlushFromFind++; //stops flush being called multiple times if this method is recursively called - try + using (SuspendAutoFlush()) //stops flush being called multiple times if this method is recursively called { return plan.PerformIterate(queryParameters, this); } - finally - { - dontFlushFromFind--; - } } } @@ -861,6 +852,34 @@ public void Delete(string entityName, object child, bool isCascadeDeleteEnabled, } } + /// + public bool AutoFlushSuspended => _suspendAutoFlushCount != 0; + + /// + public IDisposable SuspendAutoFlush() + { + return new SuspendAutoFlushHelper(this); + } + + private sealed class SuspendAutoFlushHelper : IDisposable + { + private SessionImpl _session; + + public SuspendAutoFlushHelper(SessionImpl session) + { + _session = session; + _session._suspendAutoFlushCount++; + } + + public void Dispose() + { + if (_session == null) + throw new ObjectDisposedException("The auto-flush suspension helper has been disposed already"); + _session._suspendAutoFlushCount--; + _session = null; + } + } + #endregion public object Merge(string entityName, object obj) @@ -1360,10 +1379,13 @@ public override void Flush() { throw new HibernateException("Flush during cascade is dangerous"); } - IFlushEventListener[] flushEventListener = listeners.FlushEventListeners; - for (int i = 0; i < flushEventListener.Length; i++) + using (SuspendAutoFlush()) { - flushEventListener[i].OnFlush(new FlushEvent(this)); + IFlushEventListener[] flushEventListener = listeners.FlushEventListeners; + for (int i = 0; i < flushEventListener.Length; i++) + { + flushEventListener[i].OnFlush(new FlushEvent(this)); + } } } } @@ -1611,25 +1633,26 @@ private void Filter(object collection, string filter, QueryParameters queryParam FilterQueryPlan plan = GetFilterQueryPlan(collection, filter, queryParameters, false); bool success = false; - dontFlushFromFind++; //stops flush being called multiple times if this method is recursively called - try + using (SuspendAutoFlush()) //stops flush being called multiple times if this method is recursively called { - plan.PerformList(queryParameters, this, results); - success = true; - } - catch (HibernateException) - { - // Do not call Convert on HibernateExceptions - throw; - } - catch (Exception e) - { - throw Convert(e, "could not execute query"); - } - finally - { - dontFlushFromFind--; - AfterOperation(success); + try + { + plan.PerformList(queryParameters, this, results); + success = true; + } + catch (HibernateException) + { + // Do not call Convert on HibernateExceptions + throw; + } + catch (Exception e) + { + throw Convert(e, "could not execute query"); + } + finally + { + AfterOperation(success); + } } } } @@ -1793,30 +1816,30 @@ public override void List(CriteriaImpl criteria, IList results) AutoFlushIfRequired(spaces); - dontFlushFromFind++; - bool success = false; - try + using (SuspendAutoFlush()) { - for (int i = size - 1; i >= 0; i--) + try { - ArrayHelper.AddAll(results, loaders[i].List(this)); + for (int i = size - 1; i >= 0; i--) + { + ArrayHelper.AddAll(results, loaders[i].List(this)); + } + success = true; + } + catch (HibernateException) + { + // Do not call Convert on HibernateExceptions + throw; + } + catch (Exception sqle) + { + throw Convert(sqle, "Unable to perform find"); + } + finally + { + AfterOperation(success); } - success = true; - } - catch (HibernateException) - { - // Do not call Convert on HibernateExceptions - throw; - } - catch (Exception sqle) - { - throw Convert(sqle, "Unable to perform find"); - } - finally - { - dontFlushFromFind--; - AfterOperation(success); } } } @@ -1891,16 +1914,17 @@ public override void ListCustomQuery(ICustomQuery customQuery, QueryParameters q AutoFlushIfRequired(loader.QuerySpaces); bool success = false; - dontFlushFromFind++; - try + using (SuspendAutoFlush()) { - ArrayHelper.AddAll(results, loader.List(this, queryParameters)); - success = true; - } - finally - { - dontFlushFromFind--; - AfterOperation(success); + try + { + ArrayHelper.AddAll(results, loader.List(this, queryParameters)); + success = true; + } + finally + { + AfterOperation(success); + } } } } @@ -2152,11 +2176,6 @@ public override EventListeners Listeners get { return listeners; } } - public override int DontFlushFromFind - { - get { return dontFlushFromFind; } - } - public override CacheMode CacheMode { get { return cacheMode; } diff --git a/src/NHibernate/Impl/StatelessSessionImpl.cs b/src/NHibernate/Impl/StatelessSessionImpl.cs index ce02788a1b1..f2f37fa5181 100644 --- a/src/NHibernate/Impl/StatelessSessionImpl.cs +++ b/src/NHibernate/Impl/StatelessSessionImpl.cs @@ -306,11 +306,6 @@ public override EventListeners Listeners get { throw new NotSupportedException(); } } - public override int DontFlushFromFind - { - get { return 0; } - } - public override ConnectionManager ConnectionManager { get { return connectionManager; }