Merge pull request #17 from madelson/deadlock-exception

Address #11 by throwing a specific exception type (DeadlockException)…

DistributedLock.Tests/AbstractTestCases/ExternalConnectionOrTransactionStrategyTestCases.cs

@@ -0,0 +1,64 @@
+using Medallion.Threading.Sql;
+using Medallion.Threading.Tests.Sql;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using System;
+using System.Collections.Generic;
+using System.Data.SqlClient;
+using System.Linq;
+using System.Text;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace Medallion.Threading.Tests
+{
+    [TestClass]
+    public abstract class ExternalConnectionOrTransactionStrategyTestCases<TEngineFactory, TConnectionManagementProvider> : TestBase
+        where TEngineFactory : ITestingSqlDistributedLockEngineFactory, new()
+        where TConnectionManagementProvider : TestingSqlConnectionManagementProvider, IExternalConnectionOrTransactionTestingSqlConnectionManagementProvider, new()
+    {
+        [TestMethod]
+        public void TestDeadlockDetection()
+        {
+            var timeout = TimeSpan.FromSeconds(30);
+
+            using (var engine = this.CreateEngine())
+            using (var provider = new TConnectionManagementProvider())
+            using (var barrier = new Barrier(participantCount: 2))
+            {
+                const string LockName1 = nameof(TestDeadlockDetection) + "_1",
+                    LockName2 = nameof(TestDeadlockDetection) + "_2";
+
+                Task RunDeadlock(bool isFirst)
+                {
+                    var connectionInfo = provider.GetConnectionInfo();
+                    IDistributedLock lock1, lock2;
+                    using (connectionInfo.Transaction != null ? TransactionProvider.UseTransaction(connectionInfo.Transaction) : ConnectionProvider.UseConnection(connectionInfo.Connection))
+                    {
+                        lock1 = engine.CreateLock(isFirst ? LockName1 : LockName2);
+                        lock2 = engine.CreateLock(isFirst ? LockName2 : LockName1);
+                    }
+                    return Task.Run(async () =>
+                    {
+                        using (await lock1.AcquireAsync(timeout))
+                        {
+                            barrier.SignalAndWait();
+                            (await lock2.AcquireAsync(timeout)).Dispose();
+                        }
+                    });
+                }
+
+                var tasks = new[] { RunDeadlock(isFirst: true), RunDeadlock(isFirst: false) };
+
+                Task.WhenAll(tasks).ContinueWith(_ => { }).Wait(TimeSpan.FromSeconds(10)).ShouldEqual(true, this.GetType().Name);
+
+                var deadlockVictim = tasks.Single(t => t.IsFaulted);
+                Assert.IsInstanceOfType(deadlockVictim.Exception.GetBaseException(), typeof(InvalidOperationException)); // backwards compat check
+                Assert.IsInstanceOfType(deadlockVictim.Exception.GetBaseException(), typeof(DeadlockException));
+
+                tasks.Count(t => t.Status == TaskStatus.RanToCompletion).ShouldEqual(1);
+            }
+        }
+
+        private TestingDistributedLockEngine CreateEngine() => new TEngineFactory().Create<TConnectionManagementProvider>();
+    }
+}

DistributedLock.Tests/AbstractTestCases/SqlDistributedSemaphoreSelfDeadlockTestCases.cs

@@ -27,7 +27,7 @@ public void TestSelfDeadlockThrowsOnInfiniteWait()
                 var semaphore = engine.CreateSemaphore(nameof(TestSelfDeadlockThrowsOnInfiniteWait), maxCount: 2);
                 semaphore.Acquire();
                 semaphore.Acquire();
-                var ex = TestHelper.AssertThrows<InvalidOperationException>(() => semaphore.Acquire());
+                var ex = TestHelper.AssertThrows<DeadlockException>(() => semaphore.Acquire());
                 ex.Message.Contains("Deadlock").ShouldEqual(true, ex.Message);
             }
         }

DistributedLock.Tests/DistributedLock.Tests.csproj

@@ -63,6 +63,7 @@
   </Choose>
   <ItemGroup>
     <Compile Include="AbstractTestCases\ConnectionStringStrategyTestCases.cs" />
+    <Compile Include="AbstractTestCases\ExternalConnectionOrTransactionStrategyTestCases.cs" />
     <Compile Include="AbstractTestCases\SqlDistributedSemaphoreSelfDeadlockTestCases.cs" />
     <Compile Include="Infrastructure\ActionRegistrationDisposable.cs" />
     <Compile Include="AbstractTestCases\DistributedLockCoreTestCases.cs" />
@@ -85,6 +86,7 @@
     <Compile Include="Infrastructure\TestingDistributedLockEngine.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="Infrastructure\IdleSessionKiller.cs" />
+    <Compile Include="Tests\DeadlockExceptionTest.cs" />
     <Compile Include="Tests\SqlDistributedLockTest.cs" />
     <Compile Include="Tests\SqlDistributedReaderWriterLockTest.cs" />
     <Compile Include="Tests\SqlDistributedSemaphoreTest.cs" />

DistributedLock.Tests/Tests/CombinatorialTests.cs

@@ -240,4 +240,22 @@ public class ConnectionStringStrategy_SqlSemaphoreEngineFactory_TransactionBased
 
     [TestClass]
     public class ConnectionStringStrategy_SqlSemaphoreEngineFactory_MultiplexedConnectionStringProviderTest : ConnectionStringStrategyTestCases<TestingSqlDistributedSemaphoreEngineFactory, MultiplexedConnectionStringProvider> { }
+
+    [TestClass]
+    public class ExternalConnectionOrTransactionStrategy_SqlEngineFactory_ConnectionProviderTest : ExternalConnectionOrTransactionStrategyTestCases<TestingSqlDistributedLockEngineFactory, ConnectionProvider> { }
+
+    [TestClass]
+    public class ExternalConnectionOrTransactionStrategy_SqlEngineFactory_TransactionProviderTest : ExternalConnectionOrTransactionStrategyTestCases<TestingSqlDistributedLockEngineFactory, TransactionProvider> { }
+
+    [TestClass]
+    public class ExternalConnectionOrTransactionStrategy_SqlReaderWriterEngineFactory_ConnectionProviderTest : ExternalConnectionOrTransactionStrategyTestCases<TestingSqlDistributedReaderWriterLockEngineFactory, ConnectionProvider> { }
+
+    [TestClass]
+    public class ExternalConnectionOrTransactionStrategy_SqlReaderWriterEngineFactory_TransactionProviderTest : ExternalConnectionOrTransactionStrategyTestCases<TestingSqlDistributedReaderWriterLockEngineFactory, TransactionProvider> { }
+
+    [TestClass]
+    public class ExternalConnectionOrTransactionStrategy_SqlSemaphoreEngineFactory_ConnectionProviderTest : ExternalConnectionOrTransactionStrategyTestCases<TestingSqlDistributedSemaphoreEngineFactory, ConnectionProvider> { }
+
+    [TestClass]
+    public class ExternalConnectionOrTransactionStrategy_SqlSemaphoreEngineFactory_TransactionProviderTest : ExternalConnectionOrTransactionStrategyTestCases<TestingSqlDistributedSemaphoreEngineFactory, TransactionProvider> { }
 }

DistributedLock.Tests/Tests/DeadlockExceptionTest.cs

@@ -0,0 +1,35 @@
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Runtime.Serialization.Formatters.Binary;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Medallion.Threading.Tests.Tests
+{
+    [TestClass]
+    public class DeadlockExceptionTest : TestBase
+    {
+        [TestMethod]
+        public void TestDeadlockExceptionSerialization()
+        {
+            void ThrowDeadlockException() => throw new DeadlockException(nameof(TestDeadlockExceptionSerialization), new InvalidOperationException("foo"));
+
+            DeadlockException deadlockException = null;
+            try { ThrowDeadlockException(); }
+            catch (DeadlockException ex) { deadlockException = ex; }
+
+            var formatter = new BinaryFormatter();
+            var stream = new MemoryStream();
+            formatter.Serialize(stream, deadlockException);
+
+            stream.Position = 0;
+            var deserialized = (DeadlockException)formatter.Deserialize(stream);
+            deserialized.Message.ShouldEqual(deadlockException.Message);
+            deserialized.StackTrace.ShouldEqual(deadlockException.StackTrace);
+            deserialized.InnerException.Message.ShouldEqual(deadlockException.InnerException.Message);
+        }
+    }
+}

DistributedLock.Tests/Tests/TestSetupTest.cs

@@ -3,6 +3,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Reflection;
 using System.Text;
 using System.Text.RegularExpressions;
 using System.Threading.Tasks;
@@ -90,7 +91,7 @@ private Type[] GetTypesForGenericParameter(Type genericParameter)
             var constraints = genericParameter.GetGenericParameterConstraints();
             return this.GetType().Assembly
                 .GetTypes()
-                // this doesn't support fancy constraints like class
+                // this doesn't support all fancy constraints like class or new()
                 // see https://stackoverflow.com/questions/4864496/checking-if-an-object-meets-a-generic-parameter-constraint
                 .Where(t => !t.IsAbstract && constraints.All(c => c.IsAssignableFrom(t)))
                 .SelectMany(t => t.IsGenericTypeDefinition ? this.GetTypesForGenericParameters(t.GetGenericArguments()).Select(p => t.MakeGenericType(p)) : new[] { t })

DistributedLock/DeadlockException.cs

@@ -0,0 +1,37 @@
+using System;
+using System.Collections.Generic;
+using System.Text;
+
+namespace Medallion.Threading
+{
+    /// <summary>
+    /// An exception that SOME distributed locks will throw under SOME deadlock conditions. Note that even locks
+    /// that throw this exception under some circumstances cannot detect ALL deadlock conditions
+    /// </summary>
+#if !NETSTANDARD_1_3
+    [Serializable]
+#endif
+    public sealed class DeadlockException 
+        // for backwards compat
+        : InvalidOperationException
+    {
+        /// <summary>
+        /// Constructs a new instance of <see cref="DeadlockException"/> with a default message
+        /// </summary>
+        public DeadlockException() : this("A deadlock occurred") { }
+
+        /// <summary>
+        /// Constructs an instance of <see cref="DeadlockException"/> with the given <paramref name="message"/>
+        /// </summary>
+        public DeadlockException(string message) : base(message) { }
+
+        /// <summary>
+        /// Constructs an instance of <see cref="DeadlockException"/> with the given <paramref name="message"/> and <paramref name="innerException"/>
+        /// </summary>
+        public DeadlockException(string message, Exception innerException) : base(message, innerException) { }
+
+#if !NETSTANDARD_1_3
+        private DeadlockException(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) : base(info, context) { }
+#endif
+    }
+}

DistributedLock/DistributedLock.csproj

@@ -39,8 +39,11 @@
     <NoWarn>1591</NoWarn>
   </PropertyGroup>
 
+  <PropertyGroup Condition="'$(TargetFramework)' == 'netstandard1.3'">
+    <DefineConstants>NETSTANDARD_1_3</DefineConstants>
+  </PropertyGroup>
+
   <ItemGroup Condition="'$(TargetFramework)' == 'net45'">
-
   </ItemGroup>
 
   <ItemGroup Condition="'$(TargetFramework)' == 'netstandard1.3'">

DistributedLock/Sql/SqlApplicationLock.cs

@@ -133,7 +133,7 @@ public static bool ParseExitCode(int exitCode)
                 case -2: // canceled
                     throw new OperationCanceledException(GetErrorMessage(exitCode, "canceled"));
                 case -3: // deadlock
-                    throw new InvalidOperationException(GetErrorMessage(exitCode, "deadlock"));
+                    throw new DeadlockException(GetErrorMessage(exitCode, "deadlock"));
                 case -999: // parameter / unknown
                     throw new ArgumentException(GetErrorMessage(exitCode, "parameter validation or other error"));
 

DistributedLock/Sql/SqlSemaphore.cs

@@ -162,7 +162,7 @@ private static Task<bool> ProcessAcquireResultAsync(
                     // forever but also reasonable that someone should be able to test for lock acquisition without getting a throw
                     if (timeoutMillis == Timeout.Infinite)
                     {
-                        throw new InvalidOperationException("Deadlock detected: attempt to acquire the semaphore cannot succeed because all tickets are held by the current connection");
+                        throw new DeadlockException("Deadlock detected: attempt to acquire the semaphore cannot succeed because all tickets are held by the current connection");
                     }
 
                     ticketLockName = markerTableName = null;

README.md

@@ -200,6 +200,7 @@ Most of the time, you'll want to use the default connection strategy. See more d
 - 1.4.0
 	- Added a SQL-based distributed semaphore ([#7](https://github.com/madelson/DistributedLock/issues/7))
 	- Fix bug where SqlDistributedLockConnectionStrategy.Azure would leak connections, relying on GC to reclaim them ([#14](https://github.com/madelson/DistributedLock/issues/14)). Thanks [zavalita1](https://github.com/zavalita1) for investigating this issue!
+	- Throw a specific exception type (`DeadlockException`) rather than the generic `InvalidOperationException` when a deadlock is detected ([#11](https://github.com/madelson/DistributedLock/issues/11))
 - 1.3.1 Minor fix to avoid "leaking" isolation level changes in transaction-based locks ([#8](https://github.com/madelson/DistributedLock/issues/8)). Also switched to the VS2017 project file format
 - 1.3.0 Added an Azure connection strategy to keep lock connections from becoming idle and being reclaimed by Azure's connection governor ([#5](https://github.com/madelson/DistributedLock/issues/5))
 - 1.2.0