Removed unnecessary lock statements and updated tests.

The updated test needs to refresh the lock record entity since the DB record is updated using different connections (from child lifetime scopes).

src/Orchard.Tests/Tasks/DistributedLockServiceTests.cs

@@ -1,7 +1,9 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Threading.Tasks;
 using Autofac;
+using NHibernate.Linq;
 using NUnit.Framework;
 using Orchard.Data;
 using Orchard.Environment;
@@ -18,6 +20,8 @@ namespace Orchard.Tests.Tasks {
         private StubMachineNameProvider _machineNameProvider;
         private StubThreadProvider _threadProvider;
         private IRepository<DistributedLockRecord> _distributedLockRepository;
+        private ITransactionManager _transactionManager;
+        
 
         protected override IEnumerable<Type> DatabaseTypes {
             get { yield return typeof(DistributedLockRecord); }
@@ -36,6 +40,7 @@ namespace Orchard.Tests.Tasks {
             _machineNameProvider = (StubMachineNameProvider)_container.Resolve<IMachineNameProvider>();
             _threadProvider = (StubThreadProvider)_container.Resolve<IThreadProvider>();
             _distributedLockRepository = _container.Resolve<IRepository<DistributedLockRecord>>();
+            _transactionManager = _container.Resolve<ITransactionManager>();
         }
 
         [Test]
@@ -75,11 +80,13 @@ namespace Orchard.Tests.Tasks {
 
             var lockId = Int32.Parse(@lock.Id);
             var lockRecord = _distributedLockRepository.Get(lockId);
-
+            
             _distributedLockService.ReleaseLock(@lock);
+            _session.Refresh(lockRecord);
             Assert.That(lockRecord.Count, Is.EqualTo(1));
 
             _distributedLockService.ReleaseLock(@lock);
+            _session.Refresh(lockRecord);
             Assert.That(lockRecord.Count, Is.EqualTo(0));
         }
 
@@ -193,6 +200,7 @@ namespace Orchard.Tests.Tasks {
             };
 
             _distributedLockRepository.Create(record);
+            _transactionManager.RequireNew();
             return record;
         }
 

src/Orchard/Tasks/Locking/Services/DistributedLockService.cs

@@ -16,7 +16,6 @@ namespace Orchard.Tasks.Locking.Services {
         private readonly IMachineNameProvider _machineNameProvider;
         private readonly ILifetimeScope _lifetimeScope;
         private readonly IClock _clock;
-        private readonly object _transactionManagerLock = new object();
         private readonly IThreadProvider _threadProvider;
 
         public DistributedLockService(IMachineNameProvider machineNameProvider, IThreadProvider threadProvider, ILifetimeScope lifetimeScope, IClock clock) {
@@ -43,31 +42,29 @@ namespace Orchard.Tasks.Locking.Services {
         }
 
         public void ReleaseLock(DistributedLock @lock) {
-            lock (_transactionManagerLock) {
+            var childLifetimeScope = CreateChildLifetimeScope(@lock.Name);
-                var childLifetimeScope = CreateChildLifetimeScope(@lock.Name);
 
-                try {
+            try {
-                    var repository = childLifetimeScope.Resolve<IRepository<DistributedLockRecord>>();
+                var repository = childLifetimeScope.Resolve<IRepository<DistributedLockRecord>>();
-                    var transactionManager = childLifetimeScope.Resolve<ITransactionManager>();
+                var transactionManager = childLifetimeScope.Resolve<ITransactionManager>();
-                    transactionManager.RequireNew(IsolationLevel.ReadCommitted);
+                transactionManager.RequireNew(IsolationLevel.ReadCommitted);
-                    var lockId = Int32.Parse(@lock.Id);
+                var lockId = Int32.Parse(@lock.Id);
-                    var record = repository.Get(lockId);
+                var record = repository.Get(lockId);
 
-                    if (record == null)
+                if (record == null)
-                        throw new ObjectDisposedException("@lock", "No lock record could be found for the specified lock to be released.");
+                    throw new ObjectDisposedException("@lock", "No lock record could be found for the specified lock to be released.");
 
-                    if (record.Count <= 0)
+                if (record.Count <= 0)
-                        throw new ObjectDisposedException("@lock", "The specified lock has already been released.");
+                    throw new ObjectDisposedException("@lock", "The specified lock has already been released.");
 
-                    record.Count--;
+                record.Count--;
-                }
+            }
-                catch (Exception ex) {
+            catch (Exception ex) {
-                    if (ex.IsFatal()) throw;
+                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);
+                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 {
+            finally {
-                    childLifetimeScope.Dispose();
+                childLifetimeScope.Dispose();
-                }
             }
         }
 
@@ -76,7 +73,6 @@ namespace Orchard.Tasks.Locking.Services {
             if (@lock != null)
                 return true;
 
-            Logger.Debug("Could not acquire a lock named '{0}'.", name);
             return false;
         }
 
@@ -85,7 +81,7 @@ namespace Orchard.Tasks.Locking.Services {
             if (@lock != null)
                 return @lock;
 
-            throw new TimeoutException("Could not acquire a lock within the specified amount of time.");
+            throw new TimeoutException(String.Format("Failed to acquire a lock named '{0}' within the specified timeout ('{1}').", name, timeout));
         }
 
         private DistributedLock AcquireLockInternal(string name, TimeSpan maxValidFor, string machineName, int? threadId, TimeSpan? timeout = null) {
@@ -105,11 +101,16 @@ namespace Orchard.Tasks.Locking.Services {
                 throw;
             }
 
+            Logger.Debug(timeout == null
+                ? "Failed to acquire a lock named '{0}'."
+                : "Failed to acquire a lock named '{0}' within the specified timeout ('{1}')."
+                , name, timeout);
+
             return null;
         }
 
         private DistributedLockRecord AcquireLockRecord(string name, TimeSpan maxValidFor, string machineName, int? threadId) {
-            lock (_transactionManagerLock) {
+            //lock (_transactionManagerLock) {
                 var childLifetimeScope = CreateChildLifetimeScope(name);
 
                 try {
@@ -130,9 +131,9 @@ namespace Orchard.Tasks.Locking.Services {
                     if (record != null) {
                         // Check if the machine name assigned to the lock is the one trying to acquire it.
                         if (record.MachineName == machineName) {
-                            if(record.ThreadId != threadId)
+                            if (record.ThreadId != threadId)
                                 throw new InvalidOperationException(
-                                    threadId == null 
+                                    threadId == null
                                     ? "An attempt to acquire a lock for a machine was detected while the requested lock is already assigned to a specific thread."
                                     : "An attempt to acquire a lock for a thread was detected while the requested lock is already assigned to a machine.");
 
@@ -166,7 +167,7 @@ namespace Orchard.Tasks.Locking.Services {
                 finally {
                     childLifetimeScope.Dispose();
                 }
-            }
+            //}
         }
 
         /// <summary>