From 1f777458886c521ba19d4faacb2fffdde6519f73 Mon Sep 17 00:00:00 2001 From: Matteo Piovanelli Date: Thu, 17 May 2018 21:17:44 +0200 Subject: [PATCH] Concurrency fixes (#8054) Fixes #7720 Fixes #8019 Fixes #7708 Fixes #8025 Fixes #8028 Fixes #8040 Fixes #8052 --- .../Tags/Services/TagsServiceTests.cs | 2 + .../Stubs/StubWorkContextAccessor.cs | 2 +- .../Handlers/AutoroutePartHandler.cs | 78 ++++-- .../Orchard.Autoroute.csproj | 1 + .../Services/AutorouteNoLockTableProvider.cs | 13 + .../Orchard.Tags/Services/TagService.cs | 30 ++- src/Orchard/Data/NoLockInterceptor.cs | 101 +++++++ .../Providers/DefaultNoLockTableProvider.cs | 37 +++ .../Data/Providers/INoLockTableProvider.cs | 18 ++ src/Orchard/Locking/ILockingProvider.cs | 237 +++++++++++++++++ src/Orchard/Locking/LockingProvider.cs | 251 ++++++++++++++++++ src/Orchard/Orchard.Framework.csproj | 5 + 12 files changed, 743 insertions(+), 32 deletions(-) create mode 100644 src/Orchard.Web/Modules/Orchard.Autoroute/Services/AutorouteNoLockTableProvider.cs create mode 100644 src/Orchard/Data/NoLockInterceptor.cs create mode 100644 src/Orchard/Data/Providers/DefaultNoLockTableProvider.cs create mode 100644 src/Orchard/Data/Providers/INoLockTableProvider.cs create mode 100644 src/Orchard/Locking/ILockingProvider.cs create mode 100644 src/Orchard/Locking/LockingProvider.cs diff --git a/src/Orchard.Tests.Modules/Tags/Services/TagsServiceTests.cs b/src/Orchard.Tests.Modules/Tags/Services/TagsServiceTests.cs index 79a3d935b..04b66ced6 100644 --- a/src/Orchard.Tests.Modules/Tags/Services/TagsServiceTests.cs +++ b/src/Orchard.Tests.Modules/Tags/Services/TagsServiceTests.cs @@ -11,6 +11,7 @@ using Orchard.ContentManagement.Handlers; using Orchard.ContentManagement.Records; using Orchard.Data; using Orchard.Environment; +using Orchard.Locking; using Orchard.Security; using Orchard.Tags.Handlers; using Orchard.Tags.Models; @@ -40,6 +41,7 @@ namespace Orchard.Tests.Modules.Tags.Services { builder.RegisterType().As(); builder.RegisterType().As(); builder.RegisterType().As(); + builder.RegisterType().As(); builder.RegisterGeneric(typeof(Repository<>)).As(typeof(IRepository<>)); } diff --git a/src/Orchard.Tests/Stubs/StubWorkContextAccessor.cs b/src/Orchard.Tests/Stubs/StubWorkContextAccessor.cs index 551e1a199..22707d4de 100644 --- a/src/Orchard.Tests/Stubs/StubWorkContextAccessor.cs +++ b/src/Orchard.Tests/Stubs/StubWorkContextAccessor.cs @@ -45,7 +45,7 @@ namespace Orchard.Tests.Stubs { } public string SiteName { - get { throw new NotImplementedException(); } + get { return "TestSite"; } } public string SiteSalt { diff --git a/src/Orchard.Web/Modules/Orchard.Autoroute/Handlers/AutoroutePartHandler.cs b/src/Orchard.Web/Modules/Orchard.Autoroute/Handlers/AutoroutePartHandler.cs index 1038e93ab..94d98f522 100644 --- a/src/Orchard.Web/Modules/Orchard.Autoroute/Handlers/AutoroutePartHandler.cs +++ b/src/Orchard.Web/Modules/Orchard.Autoroute/Handlers/AutoroutePartHandler.cs @@ -5,6 +5,7 @@ using Orchard.ContentManagement; using Orchard.ContentManagement.Handlers; using Orchard.Data; using Orchard.Localization; +using Orchard.Locking; using Orchard.UI.Notify; namespace Orchard.Autoroute.Handlers { @@ -13,19 +14,22 @@ namespace Orchard.Autoroute.Handlers { private readonly Lazy _autorouteService; private readonly IOrchardServices _orchardServices; private readonly IHomeAliasService _homeAliasService; + private readonly ILockingProvider _lockingProvider; public Localizer T { get; set; } public AutoroutePartHandler( IRepository autoroutePartRepository, Lazy autorouteService, - IOrchardServices orchardServices, - IHomeAliasService homeAliasService) { + IOrchardServices orchardServices, + IHomeAliasService homeAliasService, + ILockingProvider lockingProvider) { Filters.Add(StorageFilter.For(autoroutePartRepository)); _autorouteService = autorouteService; _orchardServices = orchardServices; _homeAliasService = homeAliasService; + _lockingProvider = lockingProvider; OnUpdated((ctx, part) => CreateAlias(part)); @@ -50,53 +54,74 @@ namespace Orchard.Autoroute.Handlers { }); } + private string _lockString = ""; + private string LockString { + get { + if (string.IsNullOrWhiteSpace(_lockString)) { + _lockString = string.Join( + _orchardServices.WorkContext?.CurrentSite?.BaseUrl ?? "", + _orchardServices.WorkContext?.CurrentSite?.SiteName ?? "", + "Orchard.Autoroute.Handlers.AutoroutePartHandler"); + } + + return _lockString; + } + } + private void CreateAlias(AutoroutePart part) { ProcessAlias(part); } private void PublishAlias(AutoroutePart part) { - ProcessAlias(part); + _lockingProvider.Lock(LockString, () => { + ProcessAlias(part); - // Should it become the home page? - if (part.PromoteToHomePage) { - // Get the current homepage an unmark it as the homepage. - var currentHomePage = _homeAliasService.GetHomePage(VersionOptions.Latest); - if(currentHomePage != null && currentHomePage.Id != part.Id) { - var autoroutePart = currentHomePage.As(); + // Should it become the home page? + if (part.PromoteToHomePage) { + // Get the current homepage an unmark it as the homepage. + var currentHomePage = _homeAliasService.GetHomePage(VersionOptions.Latest); + if (currentHomePage != null && currentHomePage.Id != part.Id) { + var autoroutePart = currentHomePage.As(); - if (autoroutePart != null) { - autoroutePart.PromoteToHomePage = false; - if(autoroutePart.IsPublished()) - _orchardServices.ContentManager.Publish(autoroutePart.ContentItem); + if (autoroutePart != null) { + autoroutePart.PromoteToHomePage = false; + if (autoroutePart.IsPublished()) + _orchardServices.ContentManager.Publish(autoroutePart.ContentItem); + } } + + // Update the home alias to point to this item being published. + _homeAliasService.PublishHomeAlias(part); } - // Update the home alias to point to this item being published. - _homeAliasService.PublishHomeAlias(part); - } - - _autorouteService.Value.PublishAlias(part); + _autorouteService.Value.PublishAlias(part); + }); } private void ProcessAlias(AutoroutePart part) { + LocalizedString message = null; // Generate an alias if one as not already been entered. if (String.IsNullOrWhiteSpace(part.DisplayAlias)) { part.DisplayAlias = _autorouteService.Value.GenerateAlias(part); } - // If the generated alias is empty, compute a new one. if (String.IsNullOrWhiteSpace(part.DisplayAlias)) { _autorouteService.Value.ProcessPath(part); - _orchardServices.Notifier.Warning(T("The permalink could not be generated, a new slug has been defined: \"{0}\"", part.Path)); + message = T("The permalink could not be generated, a new slug has been defined: \"{0}\"", part.Path); return; } // Check for permalink conflict, unless we are trying to set the home page. var previous = part.Path; - if (!_autorouteService.Value.ProcessPath(part)) - _orchardServices.Notifier.Warning( - T("Permalinks in conflict. \"{0}\" is already set for a previously created {2} so now it has the slug \"{1}\"", - previous, part.Path, part.ContentItem.ContentType)); + if (!_autorouteService.Value.ProcessPath(part)) { + message = + T("Permalinks in conflict. \"{0}\" is already set for a previously created {2} so now it has the slug \"{1}\"", + previous, part.Path, part.ContentItem.ContentType); + } + + if (message != null) { + _orchardServices.Notifier.Warning(message); + } } void RemoveAlias(AutoroutePart part) { @@ -106,7 +131,10 @@ namespace Orchard.Autoroute.Handlers { if (part.ContentItem.Id == homePageId) { _orchardServices.Notifier.Warning(T("You removed the content item that served as the site's home page. \nMost possibly this means that instead of the home page a \"404 Not Found\" page will be displayed. \n\nTo prevent this you can e.g. publish a content item that has the \"Set as home page\" checkbox ticked.")); } - _autorouteService.Value.RemoveAliases(part); + + _lockingProvider.Lock(LockString, () => { + _autorouteService.Value.RemoveAliases(part); + }); } } } diff --git a/src/Orchard.Web/Modules/Orchard.Autoroute/Orchard.Autoroute.csproj b/src/Orchard.Web/Modules/Orchard.Autoroute/Orchard.Autoroute.csproj index d45ce1f01..4793daeea 100644 --- a/src/Orchard.Web/Modules/Orchard.Autoroute/Orchard.Autoroute.csproj +++ b/src/Orchard.Web/Modules/Orchard.Autoroute/Orchard.Autoroute.csproj @@ -140,6 +140,7 @@ + diff --git a/src/Orchard.Web/Modules/Orchard.Autoroute/Services/AutorouteNoLockTableProvider.cs b/src/Orchard.Web/Modules/Orchard.Autoroute/Services/AutorouteNoLockTableProvider.cs new file mode 100644 index 000000000..af85027bd --- /dev/null +++ b/src/Orchard.Web/Modules/Orchard.Autoroute/Services/AutorouteNoLockTableProvider.cs @@ -0,0 +1,13 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Web; +using Orchard.Data.Providers; + +namespace Orchard.Autoroute.Services { + public class AutorouteNoLockTableProvider : INoLockTableProvider { + public IEnumerable GetTableNames() { + return new string[] { "Orchard_Autoroute_AutoroutePartRecord" }; + } + } +} \ No newline at end of file diff --git a/src/Orchard.Web/Modules/Orchard.Tags/Services/TagService.cs b/src/Orchard.Web/Modules/Orchard.Tags/Services/TagService.cs index 662f0efb4..dd30c966b 100644 --- a/src/Orchard.Web/Modules/Orchard.Tags/Services/TagService.cs +++ b/src/Orchard.Web/Modules/Orchard.Tags/Services/TagService.cs @@ -9,6 +9,7 @@ using Orchard.ContentManagement; using Orchard.Security; using Orchard.Tags.Models; using Orchard.UI.Notify; +using Orchard.Locking; namespace Orchard.Tags.Services { public class TagService : ITagService { @@ -18,19 +19,24 @@ namespace Orchard.Tags.Services { private readonly IAuthorizationService _authorizationService; private readonly IOrchardServices _orchardServices; private readonly ISessionFactoryHolder _sessionFactoryHolder; + private readonly ILockingProvider _lockingProvider; public TagService(IRepository tagRepository, IRepository contentTagRepository, INotifier notifier, IAuthorizationService authorizationService, IOrchardServices orchardServices, - ISessionFactoryHolder sessionFactoryHolder) { + ISessionFactoryHolder sessionFactoryHolder, + ILockingProvider lockingProvider) { + _tagRepository = tagRepository; _contentTagRepository = contentTagRepository; _notifier = notifier; _authorizationService = authorizationService; _orchardServices = orchardServices; _sessionFactoryHolder = sessionFactoryHolder; + _lockingProvider = lockingProvider; + Logger = NullLogger.Instance; T = NullLocalizer.Instance; } @@ -56,11 +62,23 @@ namespace Orchard.Tags.Services { } public TagRecord CreateTag(string tagName) { - var result = _tagRepository.Get(x => x.TagName == tagName); - if (result == null) { - result = new TagRecord { TagName = tagName }; - _tagRepository.Create(result); - } + TagRecord result = null; + + var lockString = string.Join(".", + _orchardServices.WorkContext?.CurrentSite?.BaseUrl ?? "", + _orchardServices.WorkContext?.CurrentSite?.SiteName ?? "", + "TagService.CreateTag", + tagName); + + _lockingProvider.Lock(lockString, + () => { + result = _tagRepository.Get(x => x.TagName == tagName); + if (result == null) { + result = new TagRecord { TagName = tagName }; + _tagRepository.Create(result); + } + }); + return result; } diff --git a/src/Orchard/Data/NoLockInterceptor.cs b/src/Orchard/Data/NoLockInterceptor.cs new file mode 100644 index 000000000..0dd3fdf45 --- /dev/null +++ b/src/Orchard/Data/NoLockInterceptor.cs @@ -0,0 +1,101 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using NHibernate; +using NHibernate.SqlCommand; +using Orchard.Data.Providers; +using Orchard.Environment.Configuration; + +namespace Orchard.Data { + public class NoLockInterceptor : EmptyInterceptor, ISessionInterceptor { + + private readonly ShellSettings _shellSettings; + private readonly IEnumerable _noLockTableProviders; + + public NoLockInterceptor( + ShellSettings shellSettings, + IEnumerable noLockTableProviders) { + + _shellSettings = shellSettings; + _noLockTableProviders = noLockTableProviders; + } + + private List _tableNames; + public List TableNames { + get { + if (_tableNames == null) { + _tableNames = new List( + _noLockTableProviders + .SelectMany(nltp => nltp.GetTableNames()) + .Distinct(StringComparer.OrdinalIgnoreCase) + .Select(n => GetPrefixedTableName(n.Trim()))); + } + return _tableNames; + } + } + + private string GetPrefixedTableName(string tableName) { + if (string.IsNullOrWhiteSpace(_shellSettings.DataTablePrefix)) { + return tableName; + } + + return _shellSettings.DataTablePrefix + "_" + tableName; + } + + // based on https://stackoverflow.com/a/39518098/2669614 + public override SqlString OnPrepareStatement(SqlString sql) { + + // Modify the sql to add hints + if (sql.StartsWithCaseInsensitive("select")) { + var parts = sql.ToString().Split().ToList(); + var fromItem = parts.FirstOrDefault(p => p.Trim().Equals("from", StringComparison.OrdinalIgnoreCase)); + int fromIndex = fromItem != null ? parts.IndexOf(fromItem) : -1; + var whereItem = parts.FirstOrDefault(p => p.Trim().Equals("where", StringComparison.OrdinalIgnoreCase)); + int whereIndex = whereItem != null ? parts.IndexOf(whereItem) : parts.Count; + + if (fromIndex == -1) + return sql; + + foreach (var tableName in TableNames) { + // set NOLOCK for each one of these tables + var tableItem = parts + .FirstOrDefault(p => p.Trim() + .Equals(tableName, StringComparison.OrdinalIgnoreCase)); + if (tableItem != null) { + // the table is involved in this statement + var tableIndex = parts.IndexOf(tableItem); + // recompute whereIndex in case we added stuff to parts + whereIndex = whereItem != null ? parts.IndexOf(whereItem) : parts.Count; + if (tableIndex > fromIndex && tableIndex < whereIndex) { // sanity check + // if before the table name we have "," or "FROM", this is not a join, but rather + // something like "FROM tableName alias ..." + // we can insert "WITH (NOLOCK)" after that + if (tableIndex == fromIndex + 1 + || parts[tableIndex - 1].Equals(",")) { + + parts.Insert(tableIndex + 2, "WITH (NOLOCK)"); + } + else { + // probably doing a join, so edit the next "on" and make it + // "WITH (NOLOCK) on" + for (int i = tableIndex + 1; i < whereIndex; i++) { + if (parts[i].Trim().Equals("on", StringComparison.OrdinalIgnoreCase)) { + parts[i] = "WITH (NOLOCK) on"; + break; + } + } + } + } + } + } + + // MUST use SqlString.Parse() method instead of new SqlString() + sql = SqlString.Parse(string.Join(" ", parts)); + } + + return sql; + } + } +} diff --git a/src/Orchard/Data/Providers/DefaultNoLockTableProvider.cs b/src/Orchard/Data/Providers/DefaultNoLockTableProvider.cs new file mode 100644 index 000000000..69ebd000f --- /dev/null +++ b/src/Orchard/Data/Providers/DefaultNoLockTableProvider.cs @@ -0,0 +1,37 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Orchard.Data.Providers { + public class DefaultNoLockTableProvider : INoLockTableProvider { + + public DefaultNoLockTableProvider() { + + // We may use AutoFac to override the default tables: + /* + + + + + + */ + TableNames = "Orchard_Framework_ContentItemVersionRecord, Title_TitlePartRecord, Orchard_Framework_ContentItemRecord"; + } + + public string TableNames { get; set; } + + private IEnumerable _tableNames; + + public IEnumerable GetTableNames() { + if (_tableNames == null) { + _tableNames = new List(TableNames + .Split(new char[] { ',', ' ' }, StringSplitOptions.RemoveEmptyEntries)); + } + return _tableNames; + } + } +} diff --git a/src/Orchard/Data/Providers/INoLockTableProvider.cs b/src/Orchard/Data/Providers/INoLockTableProvider.cs new file mode 100644 index 000000000..417d7c1cd --- /dev/null +++ b/src/Orchard/Data/Providers/INoLockTableProvider.cs @@ -0,0 +1,18 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Orchard.Data.Providers { + public interface INoLockTableProvider : IDependency { + /// + /// Returns the names of the tables from which read operations should ignore shared locks. + /// + /// An IEnumerable with the table names. + /// Implementations of this should not have to worry about table prefixes used to + /// discriminate between tenants sharing a db. That should be taken care of, if needed, where + /// the results of this method are used. Implementations of this should avoid returning null. + IEnumerable GetTableNames(); + } +} diff --git a/src/Orchard/Locking/ILockingProvider.cs b/src/Orchard/Locking/ILockingProvider.cs new file mode 100644 index 000000000..3be0645d2 --- /dev/null +++ b/src/Orchard/Locking/ILockingProvider.cs @@ -0,0 +1,237 @@ +using System; + +namespace Orchard.Locking { + public interface ILockingProvider : IDependency { + + /// + /// Handles locking on a given object to execute the desired critical code. Optionally, it is possible + /// to tell how to manage exceptions, e.g. in a case where some cleanup may be required when the + /// critical code fails after having been partially executed. + /// + /// The object upon which the lock will be created. + /// The critical code to be executed while holding the lock. + /// The (optional) action to be executed to handle exceptions while still + /// holding the lock. + /// The (optional) action to be executed to handle exceptions after the + /// lock has been released. + /// Throws an ArgumentNullException the lockOn parameter is null. + /// Internally, this method uses System.Threading.Monitor to delimit the execution of the + /// critical code. Unlike the implementation of lock(obj){}, this implementation allows handling of + /// exceptions both while holding and after releasing the lock on the object. The default behaviour + /// if both the Actions to handle exceptions are null, this method is the same as calling + /// lock(obj){criticalCode();}, meaning that it will bubble out the exception while holding the lock, and + /// only release it afterwards. If an innerHandler is provided, but outerHandler is null, an exception will + /// bubble out after the lock is released. To prevent exceptions from being thrown, both innerHandler and + /// outerHandler should be provided. + void Lock( + object lockOn, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null); + + /// + /// Handles locking on a given string to execute the desired critical code. Optionally, it is possible + /// to tell how to manage exceptions, e.g. in a case where some cleanup may be required when the + /// critical code fails after having been partially executed. + /// + /// The string upon which the lock will be created. + /// The critical code to be executed while holding the lock. + /// The (optional) action to be executed to handle exceptions while still + /// holding the lock. + /// The (optional) action to be executed to handle exceptions after the + /// lock has been released. + /// Internally, this method uses System.Threading.Monitor to delimit the execution of the + /// critical code. Unlike the implementation of lock(obj){}, this implementation allows handling of + /// exceptions both while holding and after releasing the lock on the object. The default behaviour + /// if both the Actions to handle exceptions are null, this method is the same as calling + /// lock(obj){criticalCode();}, meaning that it will bubble out the exception while holding the lock, and + /// only release it afterwards. If an innerHandler is provided, but outerHandler is null, an exception will + /// bubble out after the lock is released. To prevent exceptions from being thrown, both innerHandler and + /// outerHandler should be provided. + void Lock( + string lockOn, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null); + + /// + /// Handles locking on a given object to execute the desired critical code. Optionally, it is possible + /// to tell how to manage exceptions, e.g. in a case where some cleanup may be required when the + /// critical code fails after having been partially executed. + /// + /// The object upon which the lock will be created. + /// The critical code to be executed while holding the lock. + /// The (optional) action to be executed to handle exceptions while still + /// holding the lock. + /// The (optional) action to be executed to handle exceptions after the + /// lock has been released. + /// true if the current thread acquires the lock; otherwise, false. + /// Throws an ArgumentNullException the lockOn parameter is null. + /// Internally, this method uses System.Threading.Monitor to delimit the execution of the + /// critical code. Unlike the implementation of lock(obj){}, this implementation allows handling of + /// exceptions both while holding and after releasing the lock on the object. The default behaviour + /// if both the Actions to handle exceptions are null, this method is the same as calling + /// lock(obj){criticalCode();}, meaning that it will bubble out the exception while holding the lock, and + /// only release it afterwards. If an innerHandler is provided, but outerHandler is null, an exception will + /// bubble out after the lock is released. To prevent exceptions from being thrown, both innerHandler and + /// outerHandler should be provided. + bool TryLock( + object lockOn, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null); + + /// + /// Handles locking on a given string to execute the desired critical code. Optionally, it is possible + /// to tell how to manage exceptions, e.g. in a case where some cleanup may be required when the + /// critical code fails after having been partially executed. + /// + /// The string upon which the lock will be created. + /// The critical code to be executed while holding the lock. + /// The (optional) action to be executed to handle exceptions while still + /// holding the lock. + /// The (optional) action to be executed to handle exceptions after the + /// lock has been released. + /// true if the current thread acquires the lock; otherwise, false. + /// Internally, this method uses System.Threading.Monitor to delimit the execution of the + /// critical code. Unlike the implementation of lock(obj){}, this implementation allows handling of + /// exceptions both while holding and after releasing the lock on the object. The default behaviour + /// if both the Actions to handle exceptions are null, this method is the same as calling + /// lock(obj){criticalCode();}, meaning that it will bubble out the exception while holding the lock, and + /// only release it afterwards. If an innerHandler is provided, but outerHandler is null, an exception will + /// bubble out after the lock is released. To prevent exceptions from being thrown, both innerHandler and + /// outerHandler should be provided. + bool TryLock( + string lockOn, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null); + + /// + /// Handles locking on a given object to execute the desired critical code. Optionally, it is possible + /// to tell how to manage exceptions, e.g. in a case where some cleanup may be required when the + /// critical code fails after having been partially executed. + /// + /// The object upon which the lock will be created. + /// A TimeSpan representing the amount of time to wait for the lock. A value + /// of –1 millisecond specifies an infinite wait. + /// The critical code to be executed while holding the lock. + /// The (optional) action to be executed to handle exceptions while still + /// holding the lock. + /// The (optional) action to be executed to handle exceptions after the + /// lock has been released. + /// true if the current thread acquires the lock; otherwise, false. + /// Throws an ArgumentNullException the lockOn parameter is null. + /// Throws an ArgumentOutOfRangeException if the value of timeout + /// in milliseconds is negative and is not equal to Infinite (-1 millisecond), or is greater than MaxValue. + /// Internally, this method uses System.Threading.Monitor to delimit the execution of the + /// critical code. Unlike the implementation of lock(obj){}, this implementation allows handling of + /// exceptions both while holding and after releasing the lock on the object. The default behaviour + /// if both the Actions to handle exceptions are null, this method is the same as calling + /// lock(obj){criticalCode();}, meaning that it will bubble out the exception while holding the lock, and + /// only release it afterwards. If an innerHandler is provided, but outerHandler is null, an exception will + /// bubble out after the lock is released. To prevent exceptions from being thrown, both innerHandler and + /// outerHandler should be provided. + bool TryLock( + object lockOn, + TimeSpan timeout, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null); + + /// + /// Handles locking on a given string to execute the desired critical code. Optionally, it is possible + /// to tell how to manage exceptions, e.g. in a case where some cleanup may be required when the + /// critical code fails after having been partially executed. + /// + /// The string upon which the lock will be created. + /// A TimeSpan representing the amount of time to wait for the lock. A value + /// of –1 millisecond specifies an infinite wait. + /// The critical code to be executed while holding the lock. + /// The (optional) action to be executed to handle exceptions while still + /// holding the lock. + /// The (optional) action to be executed to handle exceptions after the + /// lock has been released. + /// true if the current thread acquires the lock; otherwise, false. + /// Throws an ArgumentNullException the lockOn parameter is null. + /// Throws an ArgumentOutOfRangeException if the value of timeout + /// in milliseconds is negative and is not equal to Infinite (-1 millisecond), or is greater than MaxValue. + /// Internally, this method uses System.Threading.Monitor to delimit the execution of the + /// critical code. Unlike the implementation of lock(obj){}, this implementation allows handling of + /// exceptions both while holding and after releasing the lock on the object. The default behaviour + /// if both the Actions to handle exceptions are null, this method is the same as calling + /// lock(obj){criticalCode();}, meaning that it will bubble out the exception while holding the lock, and + /// only release it afterwards. If an innerHandler is provided, but outerHandler is null, an exception will + /// bubble out after the lock is released. To prevent exceptions from being thrown, both innerHandler and + /// outerHandler should be provided. + bool TryLock( + string lockOn, + TimeSpan timeout, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null); + + /// + /// Handles locking on a given object to execute the desired critical code. Optionally, it is possible + /// to tell how to manage exceptions, e.g. in a case where some cleanup may be required when the + /// critical code fails after having been partially executed. + /// + /// The object upon which the lock will be created. + /// The number of milliseconds to wait for the lock.. + /// The critical code to be executed while holding the lock. + /// The (optional) action to be executed to handle exceptions while still + /// holding the lock. + /// The (optional) action to be executed to handle exceptions after the + /// lock has been released. + /// true if the current thread acquires the lock; otherwise, false. + /// Throws an ArgumentNullException the lockOn parameter is null. + /// Throws an ArgumentOutOfRangeException if the value + /// of millisecondsTimeout is negative, and not equal to Infinite. + /// Internally, this method uses System.Threading.Monitor to delimit the execution of the + /// critical code. Unlike the implementation of lock(obj){}, this implementation allows handling of + /// exceptions both while holding and after releasing the lock on the object. The default behaviour + /// if both the Actions to handle exceptions are null, this method is the same as calling + /// lock(obj){criticalCode();}, meaning that it will bubble out the exception while holding the lock, and + /// only release it afterwards. If an innerHandler is provided, but outerHandler is null, an exception will + /// bubble out after the lock is released. To prevent exceptions from being thrown, both innerHandler and + /// outerHandler should be provided. + bool TryLock( + object lockOn, + int millisecondsTimeout, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null); + + /// + /// Handles locking on a given string to execute the desired critical code. Optionally, it is possible + /// to tell how to manage exceptions, e.g. in a case where some cleanup may be required when the + /// critical code fails after having been partially executed. + /// + /// The string upon which the lock will be created. + /// The number of milliseconds to wait for the lock.. + /// The critical code to be executed while holding the lock. + /// The (optional) action to be executed to handle exceptions while still + /// holding the lock. + /// The (optional) action to be executed to handle exceptions after the + /// lock has been released. + /// true if the current thread acquires the lock; otherwise, false. + /// Throws an ArgumentNullException the lockOn parameter is null. + /// Throws an ArgumentOutOfRangeException if the value + /// of millisecondsTimeout is negative, and not equal to Infinite. + /// Internally, this method uses System.Threading.Monitor to delimit the execution of the + /// critical code. Unlike the implementation of lock(obj){}, this implementation allows handling of + /// exceptions both while holding and after releasing the lock on the object. The default behaviour + /// if both the Actions to handle exceptions are null, this method is the same as calling + /// lock(obj){criticalCode();}, meaning that it will bubble out the exception while holding the lock, and + /// only release it afterwards. If an innerHandler is provided, but outerHandler is null, an exception will + /// bubble out after the lock is released. To prevent exceptions from being thrown, both innerHandler and + /// outerHandler should be provided. + bool TryLock( + string lockOn, + int millisecondsTimeout, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null); + + } +} diff --git a/src/Orchard/Locking/LockingProvider.cs b/src/Orchard/Locking/LockingProvider.cs new file mode 100644 index 000000000..2633b639d --- /dev/null +++ b/src/Orchard/Locking/LockingProvider.cs @@ -0,0 +1,251 @@ +using System; +using System.Threading; +using Orchard.Localization; +using Orchard.Logging; + +namespace Orchard.Locking { + public class LockingProvider : ILockingProvider { + + public LockingProvider() { + Logger = NullLogger.Instance; + T = NullLocalizer.Instance; + } + + public ILogger Logger { get; set; } + public Localizer T { get; set; } + + public void Lock( + object lockOn, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null) { + + LockInternal(lockOn, criticalCode, innerHandler, outerHandler); + } + + public void Lock( + string lockOn, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null) { + + LockInternal(String.Intern(lockOn), criticalCode, innerHandler, outerHandler); + } + + public bool TryLock( + object lockOn, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null) { + + return TryLockInternal(lockOn, TimeSpan.Zero, criticalCode, innerHandler, outerHandler); + } + + public bool TryLock( + string lockOn, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null) { + + return TryLockInternal(String.Intern(lockOn), TimeSpan.Zero, criticalCode, innerHandler, outerHandler); + } + + public bool TryLock( + object lockOn, + TimeSpan timeout, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null) { + + return TryLockInternal(lockOn, timeout, criticalCode, innerHandler, outerHandler); + } + + public bool TryLock( + string lockOn, + TimeSpan timeout, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null) { + + return TryLockInternal(String.Intern(lockOn), timeout, criticalCode, innerHandler, outerHandler); + } + + public bool TryLock( + object lockOn, + int millisecondsTimeout, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null) { + + return TryLockInternal(lockOn, millisecondsTimeout, criticalCode, innerHandler, outerHandler); + } + + public bool TryLock( + string lockOn, + int millisecondsTimeout, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null) { + + return TryLockInternal(String.Intern(lockOn), millisecondsTimeout, criticalCode, innerHandler, outerHandler); + } + + private void LockInternal( + object lockOn, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null) { + + bool taken = false; + var tmp = lockOn; + Exception outerException = null; + try { + Monitor.Enter(tmp, ref taken); + criticalCode?.Invoke(); + } + catch (Exception ex) { + outerException = ex; + CleanLog(ex); + if (innerHandler != null) { + innerHandler.Invoke(ex); + } + else { + if (outerHandler == null) { + // if both the handlers are null, the methods should behave like lock(tmp){} + // and only bubble out the exception while holding the lock. + outerException = null; + } + throw ex; + } + } + finally { + if (taken) { + Monitor.Exit(tmp); + } + } + + // Even if there was an handler for the exception to be used in the critical section + // (i.e. innerHandler != null) we have further handling here. This may simply mean throwing + // the exception out when outerHandler == null + if (outerException != null) { + if (outerHandler != null) { + outerHandler.Invoke(outerException); + } + else { + throw outerException; + } + } + } + + private bool TryLockInternal( + object lockOn, + TimeSpan timeout, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null) { + + var tmp = lockOn; + Exception outerException = null; + + if (Monitor.TryEnter(tmp, timeout)) { + try { + criticalCode?.Invoke(); + } + catch (Exception ex) { + outerException = ex; + CleanLog(ex); + if (innerHandler != null) { + innerHandler.Invoke(ex); + } + else { + if (outerHandler == null) { + // if both the handlers are null, the methods should behave like lock(tmp){} + // and only bubble out the exception while holding the lock. + outerException = null; + } + throw ex; + } + } + finally { + Monitor.Exit(tmp); + } + + // Even if there was an handler for the exception to be used in the critical section + // (i.e. innerHandler != null) we have further handling here. This may simply mean throwing + // the exception out when outerHandler == null + if (outerException != null) { + if (outerHandler != null) { + outerHandler.Invoke(outerException); + } + else { + throw outerException; + } + } + + return true; + } + + return false; + } + + private bool TryLockInternal( + object lockOn, + int millisecondsTimeout, + Action criticalCode, + Action innerHandler = null, + Action outerHandler = null) { + + var tmp = lockOn; + Exception outerException = null; + + if (Monitor.TryEnter(tmp, millisecondsTimeout)) { + try { + criticalCode?.Invoke(); + } + catch (Exception ex) { + outerException = ex; + CleanLog(ex); + if (innerHandler != null) { + innerHandler.Invoke(ex); + } + else { + if (outerHandler == null) { + // if both the handlers are null, the methods should behave like lock(tmp){} + // and only bubble out the exception while holding the lock. + outerException = null; + } + throw ex; + } + } + finally { + Monitor.Exit(tmp); + } + + // Even if there was an handler for the exception to be used in the critical section + // (i.e. innerHandler != null) we have further handling here. This may simply mean throwing + // the exception out when outerHandler == null + if (outerException != null) { + if (outerHandler != null) { + outerHandler.Invoke(outerException); + } + else { + throw outerException; + } + } + + return true; + } + + return false; + } + + private void CleanLog(Exception ex) { + try { + Logger.Log(Logging.LogLevel.Error, ex, T("Exception while running critical code").Text); + } + catch (Exception) { + // prevent messing things up if the logger fails + } + } + } +} diff --git a/src/Orchard/Orchard.Framework.csproj b/src/Orchard/Orchard.Framework.csproj index f33fc42bd..8a4c988c6 100644 --- a/src/Orchard/Orchard.Framework.csproj +++ b/src/Orchard/Orchard.Framework.csproj @@ -176,6 +176,9 @@ + + + @@ -185,6 +188,8 @@ + +