From 61f1c3ae00aa3f8a25c7e6a5990ac4b565fca665 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 19 Aug 2015 23:05:39 +0100 Subject: [PATCH] Simplified distributed locking implementation and did the following: - Added Support for re-entry - Made TryAcquireLock method thread-safe. - Changed the way locks are released by decreasing their ref count. A lock is expired if: a) ref count == 0 or b) ExpiredUtc lies in the past. It is no longer necessary to delete a lock record (but would be fine, maybe as a background task). --- .../Orchard.Framework.Tests.csproj | 2 +- .../Stubs/StubDistributedLock.cs | 20 +-- src/Orchard.Tests/Tasks/DatabaseLockTests.cs | 69 -------- .../Tasks/DistributedLockServiceTests.cs | 50 +++++- src/Orchard.Tests/Tasks/LockTests.cs | 40 +++++ .../Data/Migration/AutomaticDataMigrations.cs | 40 ++--- src/Orchard/Orchard.Framework.csproj | 4 +- .../Locking/Migrations/FrameworkMigrations.cs | 8 +- .../Tasks/Locking/Providers/DatabaseLock.cs | 125 -------------- .../Locking/Records/DatabaseLockRecord.cs | 10 -- .../Tasks/Locking/Records/LockRecord.cs | 12 ++ .../Services/DistributedLockService.cs | 156 ++++++++++++++---- .../Locking/Services/IDistributedLock.cs | 14 +- .../Services/IDistributedLockService.cs | 5 + src/Orchard/Tasks/Locking/Services/Lock.cs | 31 ++++ 15 files changed, 292 insertions(+), 294 deletions(-) delete mode 100644 src/Orchard.Tests/Tasks/DatabaseLockTests.cs create mode 100644 src/Orchard.Tests/Tasks/LockTests.cs delete mode 100644 src/Orchard/Tasks/Locking/Providers/DatabaseLock.cs delete mode 100644 src/Orchard/Tasks/Locking/Records/DatabaseLockRecord.cs create mode 100644 src/Orchard/Tasks/Locking/Records/LockRecord.cs create mode 100644 src/Orchard/Tasks/Locking/Services/Lock.cs diff --git a/src/Orchard.Tests/Orchard.Framework.Tests.csproj b/src/Orchard.Tests/Orchard.Framework.Tests.csproj index ad5b95db0..7f39003cf 100644 --- a/src/Orchard.Tests/Orchard.Framework.Tests.csproj +++ b/src/Orchard.Tests/Orchard.Framework.Tests.csproj @@ -292,7 +292,7 @@ - + diff --git a/src/Orchard.Tests/Stubs/StubDistributedLock.cs b/src/Orchard.Tests/Stubs/StubDistributedLock.cs index 6944b2bc6..e02d7883d 100644 --- a/src/Orchard.Tests/Stubs/StubDistributedLock.cs +++ b/src/Orchard.Tests/Stubs/StubDistributedLock.cs @@ -3,25 +3,13 @@ using Orchard.Tasks.Locking.Services; namespace Orchard.Tests.Stubs { public class StubDistributedLock : IDistributedLock { - public static bool IsAcquired { get; private set; } - public static string AcquiredByMachineName { get; private set; } - public bool IsDisposed { get; private set; } - - public bool TryAcquire(string name, string machineName, TimeSpan maxLifetime) { - if (IsAcquired && machineName != AcquiredByMachineName) - return false; - - IsAcquired = true; - AcquiredByMachineName = machineName; - return true; - } - - + public void Dispose() { IsDisposed = true; - IsAcquired = false; - AcquiredByMachineName = null; } + + public int Id { get; set; } + public string Name { get; set; } } } diff --git a/src/Orchard.Tests/Tasks/DatabaseLockTests.cs b/src/Orchard.Tests/Tasks/DatabaseLockTests.cs deleted file mode 100644 index ae7114626..000000000 --- a/src/Orchard.Tests/Tasks/DatabaseLockTests.cs +++ /dev/null @@ -1,69 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using Autofac; -using NUnit.Framework; -using Orchard.Data; -using Orchard.Tasks.Locking.Providers; -using Orchard.Tasks.Locking.Records; - -namespace Orchard.Tests.Tasks { - [TestFixture] - public class DatabaseLockTests : DatabaseEnabledTestsBase { - private const string MachineName1 = "Orchard Testing Machine 1"; - private const string MachineName2 = "Orchard Testing Machine 2"; - private const string LockName = "Orchard Test Lock"; - private IRepository _databaseLockRepository; - private DatabaseLock _lock; - - protected override IEnumerable DatabaseTypes { - get { - yield return typeof(DatabaseLockRecord); - } - } - - public override void Register(ContainerBuilder builder) { - builder.RegisterType().AsSelf(); - } - - public override void Init() { - base.Init(); - - _databaseLockRepository = _container.Resolve>(); - _lock = _container.Resolve(); - } - - [Test] - public void AcquiringLockSucceeds() { - var lockAcquired = _lock.TryAcquire(LockName, MachineName1, TimeSpan.FromSeconds(60)); - - Assert.That(lockAcquired, Is.True); - Assert.That(_databaseLockRepository.Table.Count(), Is.EqualTo(1)); - } - - [Test] - public void DisposingRemovesLockRecord() { - _lock.TryAcquire(LockName, MachineName1, TimeSpan.FromSeconds(60)); - _lock.Dispose(); - Assert.That(_databaseLockRepository.Table.Count(), Is.EqualTo(0)); - } - - [Test] - public void AcquiringLockTwiceFails() { - var attempt1 = _lock.TryAcquire(LockName, MachineName1, TimeSpan.FromSeconds(60)); - var attempt2 = _lock.TryAcquire(LockName, MachineName2, TimeSpan.FromSeconds(60)); - - Assert.That(attempt1, Is.True); - Assert.That(attempt2, Is.False); - } - - [Test] - public void AcquiringExpiredLockSucceeds() { - var attempt1 = _lock.TryAcquire(LockName, MachineName1, TimeSpan.FromSeconds(60)); - var attempt2 = _lock.TryAcquire(LockName, MachineName2, TimeSpan.FromSeconds(-1)); // Treat the previosuly stored lock as immediately expired. - - Assert.That(attempt1, Is.True); - Assert.That(attempt2, Is.True); - } - } -} \ No newline at end of file diff --git a/src/Orchard.Tests/Tasks/DistributedLockServiceTests.cs b/src/Orchard.Tests/Tasks/DistributedLockServiceTests.cs index 7e9f547fe..c7d2bed92 100644 --- a/src/Orchard.Tests/Tasks/DistributedLockServiceTests.cs +++ b/src/Orchard.Tests/Tasks/DistributedLockServiceTests.cs @@ -4,28 +4,40 @@ using System.Linq; using System.Threading.Tasks; using Autofac; using NUnit.Framework; +using Orchard.Data; using Orchard.Environment; +using Orchard.Services; +using Orchard.Tasks.Locking.Records; using Orchard.Tasks.Locking.Services; using Orchard.Tests.Stubs; namespace Orchard.Tests.Tasks { [TestFixture] - public class DistributedLockServiceTests : ContainerTestBase { + public class DistributedLockServiceTests : DatabaseEnabledTestsBase { private const string LockName = "Orchard Test Lock"; private DistributedLockService _distributedLockService; private StubMachineNameProvider _stubMachineNameProvider; + private IRepository _lockRepository; - protected override void Register(ContainerBuilder builder) { - builder.RegisterType().As(); + protected override IEnumerable DatabaseTypes + { + get { yield return typeof (LockRecord); } + } + + public override void Register(ContainerBuilder builder) { + builder.RegisterType().As(); + //builder.RegisterType().As(); builder.RegisterType().As().SingleInstance(); builder.RegisterType().As(); builder.RegisterType().AsSelf(); builder.RegisterInstance(new Work(resolve => _container.Resolve())).AsSelf(); } - protected override void Resolve(ILifetimeScope container) { - _distributedLockService = container.Resolve(); - _stubMachineNameProvider = (StubMachineNameProvider)container.Resolve(); + public override void Init() { + base.Init(); + _distributedLockService = _container.Resolve(); + _stubMachineNameProvider = (StubMachineNameProvider)_container.Resolve(); + _lockRepository = _container.Resolve>(); } @@ -47,6 +59,30 @@ namespace Orchard.Tests.Tasks { Assert.That(attempt2, Is.True); } + [Test] + public void AcquiringLockTwiceOnSameMachineIncreasesRefCountTwice() { + IDistributedLock @lock; + _distributedLockService.TryAcquireLock(LockName, TimeSpan.FromSeconds(60), TimeSpan.Zero, out @lock); + _distributedLockService.TryAcquireLock(LockName, TimeSpan.FromSeconds(60), TimeSpan.Zero, out @lock); + + var lockRecord = _lockRepository.Get(@lock.Id); + Assert.That(lockRecord.ReferenceCount, Is.EqualTo(2)); + } + + [Test] + public void DisposingLockWillDecreaseRefCount() { + IDistributedLock @lock; + _distributedLockService.TryAcquireLock(LockName, TimeSpan.FromSeconds(60), TimeSpan.Zero, out @lock); + _distributedLockService.TryAcquireLock(LockName, TimeSpan.FromSeconds(60), TimeSpan.Zero, out @lock); + var lockRecord = _lockRepository.Get(@lock.Id); + + _distributedLockService.DisposeLock(@lock); + Assert.That(lockRecord.ReferenceCount, Is.EqualTo(1)); + + _distributedLockService.DisposeLock(@lock); + Assert.That(lockRecord.ReferenceCount, Is.EqualTo(0)); + } + [Test] public void AcquiringLockTwiceFails() { IDistributedLock @lock; @@ -64,7 +100,7 @@ namespace Orchard.Tests.Tasks { var attempts = new List(); var tasks = new List(); - foreach (var index in Enumerable.Range(0, 10).AsParallel()) { + foreach (var index in Enumerable.Range(0, 20)) { var task = Task.Factory.StartNew(() => { IDistributedLock @lock; _stubMachineNameProvider.MachineName = "Orchard Test Machine " + (index + 1); diff --git a/src/Orchard.Tests/Tasks/LockTests.cs b/src/Orchard.Tests/Tasks/LockTests.cs new file mode 100644 index 000000000..eaa09e9e4 --- /dev/null +++ b/src/Orchard.Tests/Tasks/LockTests.cs @@ -0,0 +1,40 @@ +using System; +using Autofac; +using Moq; +using NUnit.Framework; +using Orchard.Tasks.Locking.Services; + +namespace Orchard.Tests.Tasks { + [TestFixture] + public class LockTests : ContainerTestBase { + private const string LockName = "Orchard Test Lock"; + private const int LockId = 1; + private Mock _distributedLockServiceMock; + private Lock _lock; + + protected override void Register(ContainerBuilder builder) { + _distributedLockServiceMock = new Mock(); + builder.RegisterInstance(_distributedLockServiceMock.Object); + } + + protected override void Resolve(ILifetimeScope container) { + _lock = new Lock(_distributedLockServiceMock.Object, LockName, LockId); + } + + [Test] + public void DisposeInvokesDistributedLockServiceDisposeLock() { + _lock.Dispose(); + + _distributedLockServiceMock.Verify(service => service.DisposeLock(_lock), Times.Exactly(1)); + } + + [Test] + public void DisposingMultipleTimesInvokesDistributedLockServiceDisposeLockOnce() { + _lock.Dispose(); + _lock.Dispose(); + _lock.Dispose(); + + _distributedLockServiceMock.Verify(service => service.DisposeLock(_lock), Times.Exactly(1)); + } + } +} \ No newline at end of file diff --git a/src/Orchard/Data/Migration/AutomaticDataMigrations.cs b/src/Orchard/Data/Migration/AutomaticDataMigrations.cs index aa55ff106..26cd8bc21 100644 --- a/src/Orchard/Data/Migration/AutomaticDataMigrations.cs +++ b/src/Orchard/Data/Migration/AutomaticDataMigrations.cs @@ -29,34 +29,34 @@ namespace Orchard.Data.Migration { public ILogger Logger { get; set; } public void Activated() { - IDistributedLock @lock; - if (_distributedLockService.TryAcquireLock(GetType().FullName, TimeSpan.FromMinutes(30), TimeSpan.FromMilliseconds(250), out @lock)) { - using (@lock) { - // Let's make sure that the basic set of features is enabled. If there are any that are not enabled, then let's enable them first. - var theseFeaturesShouldAlwaysBeActive = new[] { - "Common", "Containers", "Contents", "Dashboard", "Feeds", "Navigation", "Scheduling", "Settings", "Shapes", "Title" - }; + using(var @lock = _distributedLockService.AcquireLock(GetType().FullName, TimeSpan.FromMinutes(30), TimeSpan.FromMilliseconds(250))) { + if (@lock == null) + return; - var enabledFeatures = _featureManager.GetEnabledFeatures().Select(f => f.Id).ToList(); - var featuresToEnable = theseFeaturesShouldAlwaysBeActive.Where(shouldBeActive => !enabledFeatures.Contains(shouldBeActive)).ToList(); - if (featuresToEnable.Any()) { - _featureManager.EnableFeatures(featuresToEnable, true); + // Let's make sure that the basic set of features is enabled. If there are any that are not enabled, then let's enable them first. + var theseFeaturesShouldAlwaysBeActive = new[] { + "Common", "Containers", "Contents", "Dashboard", "Feeds", "Navigation", "Scheduling", "Settings", "Shapes", "Title" + }; + + var enabledFeatures = _featureManager.GetEnabledFeatures().Select(f => f.Id).ToList(); + var featuresToEnable = theseFeaturesShouldAlwaysBeActive.Where(shouldBeActive => !enabledFeatures.Contains(shouldBeActive)).ToList(); + if (featuresToEnable.Any()) { + _featureManager.EnableFeatures(featuresToEnable, true); + } + + foreach (var feature in _dataMigrationManager.GetFeaturesThatNeedUpdate()) { + try { + _dataMigrationManager.Update(feature); } - - foreach (var feature in _dataMigrationManager.GetFeaturesThatNeedUpdate()) { - try { - _dataMigrationManager.Update(feature); - } - catch (Exception e) { - Logger.Error("Could not run migrations automatically on " + feature, e); - } + catch (Exception e) { + Logger.Error("Could not run migrations automatically on " + feature, e); } } } } public void Terminating() { - + // No-op. } } } diff --git a/src/Orchard/Orchard.Framework.csproj b/src/Orchard/Orchard.Framework.csproj index 34f3d91ae..d75ad7574 100644 --- a/src/Orchard/Orchard.Framework.csproj +++ b/src/Orchard/Orchard.Framework.csproj @@ -398,11 +398,11 @@ - + - + diff --git a/src/Orchard/Tasks/Locking/Migrations/FrameworkMigrations.cs b/src/Orchard/Tasks/Locking/Migrations/FrameworkMigrations.cs index 31d831b63..925b98c86 100644 --- a/src/Orchard/Tasks/Locking/Migrations/FrameworkMigrations.cs +++ b/src/Orchard/Tasks/Locking/Migrations/FrameworkMigrations.cs @@ -5,11 +5,13 @@ namespace Orchard.Tasks.Locking.Migrations { public class FrameworkMigrations : DataMigrationImpl { public int Create() { - SchemaBuilder.CreateTable("DatabaseLockRecord", table => table + SchemaBuilder.CreateTable("LockRecord", table => table .Column("Id", column => column.PrimaryKey().Identity()) .Column("Name", column => column.NotNull().WithLength(256)) - .Column("MachineName", column => column.WithLength(256)) - .Column("AcquiredUtc")); + .Column("Owner", column => column.WithLength(256)) + .Column("ReferenceCount") + .Column("CreatedUtc") + .Column("ValidUntilUtc")); return 1; } diff --git a/src/Orchard/Tasks/Locking/Providers/DatabaseLock.cs b/src/Orchard/Tasks/Locking/Providers/DatabaseLock.cs deleted file mode 100644 index 28553bd52..000000000 --- a/src/Orchard/Tasks/Locking/Providers/DatabaseLock.cs +++ /dev/null @@ -1,125 +0,0 @@ -using System; -using System.Data; -using System.Linq; -using Autofac; -using Orchard.Data; -using Orchard.Exceptions; -using Orchard.Logging; -using Orchard.Services; -using Orchard.Tasks.Locking.Records; -using Orchard.Tasks.Locking.Services; -using Orchard.Validation; - -namespace Orchard.Tasks.Locking.Providers { - - /// - /// Provides a database driven implementation of - /// - public class DatabaseLock : IDistributedLock { - private readonly ILifetimeScope _lifetimeScope; - private readonly IClock _clock; - private string _name; - private bool _isAcquired; - private int _id; - private bool _isDisposed; - private ILifetimeScope _scope; - - public DatabaseLock(ILifetimeScope lifetimeScope, IClock clock) { - _lifetimeScope = lifetimeScope; - _clock = clock; - Logger = NullLogger.Instance; - } - - public ILogger Logger { get; set; } - - public bool TryAcquire(string name, string machineName, TimeSpan maxLifetime) { - if(_isDisposed) - throw new ObjectDisposedException("DatabaseLock"); - - Argument.ThrowIfNullOrEmpty(name, "name"); - - if (name.Length > 256) - throw new ArgumentException("The lock's name can't be longer than 256 characters."); - - try { - var scope = EnsureLifetimeScope(name); - scope.Resolve().RequireNew(IsolationLevel.ReadCommitted); - - // This way we can create a nested transaction scope instead of having the unwanted effect - // of manipulating the transaction of the caller. - var repository = scope.Resolve>(); - var record = repository.Table.FirstOrDefault(x => x.Name == name); - - if (record != null) { - // There is an existing lock, but check if it has expired or if the current machine is requesting the lock. - var isExpired = record.AcquiredUtc + maxLifetime < _clock.UtcNow; - var isCurrentMachine = record.MachineName == machineName; - if (isExpired || isCurrentMachine) { - repository.Delete(record); - record = null; - } - } - - var canAcquire = record == null; - - if (canAcquire) { - record = new DatabaseLockRecord { - Name = name, - MachineName = machineName, - AcquiredUtc = _clock.UtcNow - }; - repository.Create(record); - repository.Flush(); - - _name = name; - _isAcquired = true; - _id = record.Id; - } - - return canAcquire; - } - catch (Exception ex) { - Logger.Error(ex, "An error occurred while trying to acquire a lock."); - DisposeLifetimeScope(); - throw; - } - } - - // This will be called at least and at the latest by the IoC container when the request ends. - public void Dispose() { - if (_scope == null) - return; - - if (!_isDisposed) { - _isDisposed = true; - - if (_isAcquired) { - try { - var repository = _scope.Resolve>(); - var record = repository.Get(_id); - - if (record != null) { - repository.Delete(record); - repository.Flush(); - } - } - catch (Exception ex) { - if (ex.IsFatal()) throw; - Logger.Error(ex, "An non-fatal error occurred while trying to dispose the database lock."); - } - } - - _scope.Dispose(); - } - } - - private ILifetimeScope EnsureLifetimeScope(string name) { - return _scope ?? (_scope = _lifetimeScope.BeginLifetimeScope("Orchard.Tasks.Locking.Database." + name)); - } - - private void DisposeLifetimeScope() { - if (_scope != null) - _scope.Dispose(); - } - } -} \ No newline at end of file diff --git a/src/Orchard/Tasks/Locking/Records/DatabaseLockRecord.cs b/src/Orchard/Tasks/Locking/Records/DatabaseLockRecord.cs deleted file mode 100644 index c8bf06828..000000000 --- a/src/Orchard/Tasks/Locking/Records/DatabaseLockRecord.cs +++ /dev/null @@ -1,10 +0,0 @@ -using System; - -namespace Orchard.Tasks.Locking.Records { - public class DatabaseLockRecord { - public virtual int Id { get; set; } - public virtual string Name { get; set; } - public virtual string MachineName { get; set; } - public virtual DateTime? AcquiredUtc { get; set; } - } -} \ No newline at end of file diff --git a/src/Orchard/Tasks/Locking/Records/LockRecord.cs b/src/Orchard/Tasks/Locking/Records/LockRecord.cs new file mode 100644 index 000000000..9cb44d3de --- /dev/null +++ b/src/Orchard/Tasks/Locking/Records/LockRecord.cs @@ -0,0 +1,12 @@ +using System; + +namespace Orchard.Tasks.Locking.Records { + public class LockRecord { + public virtual int Id { get; set; } + public virtual string Name { get; set; } + public virtual string Owner { get; set; } + public virtual int ReferenceCount { get; set; } + public virtual DateTime CreatedUtc { get; set; } + public virtual DateTime ValidUntilUtc { get; set; } + } +} \ No newline at end of file diff --git a/src/Orchard/Tasks/Locking/Services/DistributedLockService.cs b/src/Orchard/Tasks/Locking/Services/DistributedLockService.cs index c1566a782..e2211980a 100644 --- a/src/Orchard/Tasks/Locking/Services/DistributedLockService.cs +++ b/src/Orchard/Tasks/Locking/Services/DistributedLockService.cs @@ -1,59 +1,153 @@ using System; +using System.Data; +using System.Linq; using System.Threading; using System.Threading.Tasks; +using Autofac; +using Orchard.Data; using Orchard.Environment; +using Orchard.Exceptions; using Orchard.Logging; +using Orchard.Services; +using Orchard.Tasks.Locking.Records; namespace Orchard.Tasks.Locking.Services { - public class DistributedLockService : IDistributedLockService { - private readonly IWorkContextAccessor _wca; - private readonly IMachineNameProvider _machineNameProvider; - public DistributedLockService(IWorkContextAccessor wca, IMachineNameProvider machineNameProvider) { - _wca = wca; + public class DistributedLockService : IDistributedLockService { + private readonly IMachineNameProvider _machineNameProvider; + private readonly ILifetimeScope _lifetimeScope; + private readonly IClock _clock; + private readonly object _semaphore = new object(); + + public DistributedLockService(IMachineNameProvider machineNameProvider, ILifetimeScope lifetimeScope, IClock clock) { _machineNameProvider = machineNameProvider; + _lifetimeScope = lifetimeScope; + _clock = clock; Logger = NullLogger.Instance; } public ILogger Logger { get; set; } public bool TryAcquireLock(string name, TimeSpan maxLifetime, TimeSpan timeout, out IDistributedLock @lock) { - var machineName = _machineNameProvider.GetMachineName(); - @lock = Resolve(); + lock (_semaphore) { + @lock = default(IDistributedLock); - try { - var waitedTime = TimeSpan.Zero; - var waitTime = TimeSpan.FromMilliseconds(timeout.TotalMilliseconds / 10); - bool acquired; + try { + var waitedTime = TimeSpan.Zero; + var waitTime = TimeSpan.FromMilliseconds(timeout.TotalMilliseconds / 10); + bool acquired; - while (!(acquired = @lock.TryAcquire(name, machineName, maxLifetime)) && waitedTime < timeout) { - Task.Delay(timeout).ContinueWith(t => { - waitedTime += waitTime; - }).Wait(); + while (!(acquired = TryAcquireLockRecord(name, maxLifetime, out @lock)) && waitedTime < timeout) { + Task.Delay(timeout).ContinueWith(t => { + waitedTime += waitTime; + }).Wait(); + } + + if (acquired) { + Logger.Debug("Successfully acquired a lock named {0}.", name); + return true; + } + } + catch (Exception ex) { + Logger.Error(ex, "Error while trying to acquire a lock named {0}.", name); + throw; } - if (acquired) { - Logger.Debug("Successfully acquired a lock named {0} on machine {1}.", name, machineName); - return true; - } + Logger.Debug("Could not acquire a lock named {0}.", name); + return false; } - catch (Exception ex) { - Logger.Error(ex, "Error while trying to acquire a lock named {0} on machine {1}.", name, machineName); - throw; - } - - Logger.Debug("Could not acquire a lock named {0} on machine {1}.", name, machineName); - return false; - } - - private T Resolve() { - var workContext = _wca.GetContext() ?? _wca.CreateWorkContextScope().WorkContext; // In case this is invoked at the end of the request. - return workContext.Resolve(); } public IDistributedLock AcquireLock(string name, TimeSpan maxLifetime, TimeSpan timeout) { IDistributedLock lockResult; return TryAcquireLock(name, maxLifetime, timeout, out lockResult) ? lockResult : null; } + + public void DisposeLock(IDistributedLock @lock) { + var childLifetimeScope = CreateChildLifetimeScope(@lock.Name); + + try { + var repository = childLifetimeScope.Resolve>(); + var transactionManager = childLifetimeScope.Resolve(); + transactionManager.RequireNew(IsolationLevel.ReadCommitted); + var record = repository.Get(@lock.Id); + + if (record != null) { + if (record.ReferenceCount > 0) + record.ReferenceCount--; + } + } + catch (Exception ex) { + if (ex.IsFatal()) throw; + Logger.Error(ex, "An non-fatal error occurred while trying to dispose a distributed lock with name {0} and ID {1}.", @lock.Name, @lock.Id); + } + finally { + childLifetimeScope.Dispose(); + } + } + + protected virtual bool TryAcquireLockRecord(string name, TimeSpan maxLifetime, out IDistributedLock @lock) { + @lock = null; + var childLifetimeScope = CreateChildLifetimeScope(name); + + try { + var transactionManager = childLifetimeScope.Resolve(); + transactionManager.RequireNew(IsolationLevel.ReadCommitted); + + // This way we can create a nested transaction scope instead of having the unwanted effect + // of manipulating the transaction of the caller. + var repository = childLifetimeScope.Resolve>(); + + // Find an existing, active lock, if any. + var record = repository.Table.FirstOrDefault(x => x.Name == name && (x.ValidUntilUtc >= _clock.UtcNow || x.ReferenceCount > 0)); + + // The current owner name (based on machine name and current thread ID). + var currentOwnerName = GetOwnerName(); + var canAcquireLock = false; + + // Check if there's already an active lock. + if (record != null) { + // Check if the owner of the lock is the one trying to acquire the lock. + if (record.Owner == currentOwnerName) { + record.ReferenceCount++; + canAcquireLock = true; + } + } + else { + // No one has an active lock yet, so good to go. + record = new LockRecord { + Name = name, + Owner = currentOwnerName, + ReferenceCount = 1, + CreatedUtc = _clock.UtcNow, + ValidUntilUtc = _clock.UtcNow + maxLifetime + }; + repository.Create(record); + repository.Flush(); + canAcquireLock = true; + } + + if (!canAcquireLock) + return false; + + @lock = new Lock(this, name, record.Id); + return true; + } + catch (Exception ex) { + Logger.Error(ex, "An error occurred while trying to acquire a lock."); + throw; + } + finally { + childLifetimeScope.Dispose(); + } + } + + private string GetOwnerName() { + return String.Format("{0}_{1}", _machineNameProvider.GetMachineName(), Thread.CurrentThread.ManagedThreadId); + } + + private ILifetimeScope CreateChildLifetimeScope(string name) { + return _lifetimeScope.BeginLifetimeScope("Orchard.Tasks.Locking." + name); + } } } \ No newline at end of file diff --git a/src/Orchard/Tasks/Locking/Services/IDistributedLock.cs b/src/Orchard/Tasks/Locking/Services/IDistributedLock.cs index dcbc2e7b9..e38fe5da2 100644 --- a/src/Orchard/Tasks/Locking/Services/IDistributedLock.cs +++ b/src/Orchard/Tasks/Locking/Services/IDistributedLock.cs @@ -2,16 +2,10 @@ namespace Orchard.Tasks.Locking.Services { /// - /// Provides a lock on a provided name. + /// Represents a distributed lock. /// - public interface IDistributedLock : ITransientDependency, IDisposable { - /// - /// Tries to acquire a lock on the specified name. - /// - /// The name to use for the lock. - /// The machine name trying to acquire a lock. - /// The maximum amount of time the lock is allowed. This is a safety net in case the caller fails to release the lock. - /// Returns true if a lock was acquired, false otherwise. - bool TryAcquire(string name, string machineName, TimeSpan maxLifetime); + public interface IDistributedLock : IDisposable { + int Id { get; } + string Name { get; } } } \ No newline at end of file diff --git a/src/Orchard/Tasks/Locking/Services/IDistributedLockService.cs b/src/Orchard/Tasks/Locking/Services/IDistributedLockService.cs index 98e1992e0..1ba6d9060 100644 --- a/src/Orchard/Tasks/Locking/Services/IDistributedLockService.cs +++ b/src/Orchard/Tasks/Locking/Services/IDistributedLockService.cs @@ -23,5 +23,10 @@ namespace Orchard.Tasks.Locking.Services { /// The amount of time to wait for the lock to be acquired before timing out. /// Returns a lock if one was successfully acquired, null otherwise. IDistributedLock AcquireLock(string name, TimeSpan maxLifetime, TimeSpan timeout); + + /// + /// Disposes the specified lock. + /// + void DisposeLock(IDistributedLock @lock); } } \ No newline at end of file diff --git a/src/Orchard/Tasks/Locking/Services/Lock.cs b/src/Orchard/Tasks/Locking/Services/Lock.cs new file mode 100644 index 000000000..21a2eca42 --- /dev/null +++ b/src/Orchard/Tasks/Locking/Services/Lock.cs @@ -0,0 +1,31 @@ +using Orchard.Logging; + +namespace Orchard.Tasks.Locking.Services { + + /// + /// Provides a database driven implementation of + /// + public class Lock : IDistributedLock { + private readonly IDistributedLockService _distributedLockService; + public string Name { get; set; } + private bool _isDisposed; + + public Lock(IDistributedLockService distributedLockService, string name, int id) { + _distributedLockService = distributedLockService; + Name = name; + Id = id; + } + + public ILogger Logger { get; set; } + public int Id { get; set; } + + // This will be called at least and at the latest by the IoC container when the request ends. + public void Dispose() { + if (!_isDisposed) { + _isDisposed = true; + + _distributedLockService.DisposeLock(this); + } + } + } +} \ No newline at end of file