Addresses possible deadlock in BufferReadWrite and SharedArray (fixes #…

…21)

SharedMemory/BufferWithLocks.cs

@@ -76,6 +76,24 @@ protected BufferWithLocks(string name, long bufferSize, bool ownsSharedMemory)
 
         #region Synchronisation
 
+        private int _readWriteTimeout = 100;
+
+        /// <summary>
+        /// The Read/Write operation timeout in milliseconds (to prevent deadlocks). Defaults to 100ms and must be larger than -1.
+        /// If a Read or Write operation's WaitEvent does not complete within this timeframe a <see cref="TimeoutException"/> will be thrown.
+        /// If using AcquireReadLock/ReleaseReadLock and AcquireWriteLock/ReleaseWriteLock correctly this timeout will never occur.
+        /// </summary>
+        public virtual int ReadWriteTimeout
+        {
+            get { return _readWriteTimeout; }
+            set
+            {
+                if (value < 0)
+                    throw new ArgumentOutOfRangeException("ReadWriteTimeout", "Must be larger than -1.");
+                _readWriteTimeout = value;
+            }
+        }
+
         /// <summary>
         /// Blocks the current thread until it is able to acquire a read lock. If successful all subsequent writes will be blocked until after a call to <see cref="ReleaseReadLock"/>.
         /// </summary>
@@ -126,6 +144,15 @@ public void ReleaseWriteLock()
 
         #region Writing
 
+        /// <summary>
+        /// Prevents write operations from deadlocking by throwing a TimeoutException if the WriteWaitEvent is not available within <see cref="ReadWriteTimeout"/> milliseconds
+        /// </summary>
+        private void WriteWait()
+        {
+            if (!WriteWaitEvent.WaitOne(ReadWriteTimeout))
+                throw new TimeoutException("The write operation timed out waiting for the write lock WaitEvent. Check your usage of AcquireWriteLock/ReleaseWriteLock and AcquireReadLock/ReleaseReadLock.");
+        }
+
         /// <summary>
         /// Writes an instance of <typeparamref name="T"/> into the buffer
         /// </summary>
@@ -134,7 +161,7 @@ public void ReleaseWriteLock()
         /// <param name="bufferPosition">The offset within the buffer region of the shared memory to write to.</param>
         protected override void Write<T>(ref T data, long bufferPosition = 0)
         {
-            WriteWaitEvent.WaitOne();
+            WriteWait();
             base.Write<T>(ref data, bufferPosition);
         }
 
@@ -146,7 +173,7 @@ protected override void Write<T>(ref T data, long bufferPosition = 0)
         /// <param name="bufferPosition">The offset within the buffer region of the shared memory to write to.</param>
         protected override void Write<T>(T[] buffer, long bufferPosition = 0)
         {
-            WriteWaitEvent.WaitOne();
+            WriteWait();
             base.Write<T>(buffer, bufferPosition);
         }
 
@@ -158,7 +185,7 @@ protected override void Write<T>(T[] buffer, long bufferPosition = 0)
         /// <param name="bufferPosition">The offset within the buffer region of the shared memory to write to.</param>
         protected override void Write(IntPtr ptr, int length, long bufferPosition = 0)
         {
-            WriteWaitEvent.WaitOne();
+            WriteWait();
             base.Write(ptr, length, bufferPosition);
         }
 
@@ -169,14 +196,23 @@ protected override void Write(IntPtr ptr, int length, long bufferPosition = 0)
         /// <param name="bufferPosition">The offset within the buffer region to start writing from.</param>
         protected override void Write(Action<IntPtr> writeFunc, long bufferPosition = 0)
         {
-            WriteWaitEvent.WaitOne();
+            WriteWait();
             base.Write(writeFunc, bufferPosition);
         }
 
         #endregion
 
         #region Reading
 
+        /// <summary>
+        /// Prevents read operations from deadlocking by throwing a TimeoutException if the ReadWaitEvent is not available within <see cref="ReadWriteTimeout"/> milliseconds
+        /// </summary>
+        private void ReadWait()
+        {
+            if (!ReadWaitEvent.WaitOne(ReadWriteTimeout))
+                throw new TimeoutException("The read operation timed out waiting for the read lock WaitEvent. Check your usage of AcquireWriteLock/ReleaseWriteLock and AcquireReadLock/ReleaseReadLock.");
+        }
+
         /// <summary>
         /// Reads an instance of <typeparamref name="T"/> from the buffer
         /// </summary>
@@ -185,7 +221,7 @@ protected override void Write(Action<IntPtr> writeFunc, long bufferPosition = 0)
         /// <param name="bufferPosition">The offset within the buffer region of the shared memory to read from.</param>
         protected override void Read<T>(out T data, long bufferPosition = 0)
         {
-            ReadWaitEvent.WaitOne();
+            ReadWait();
             base.Read<T>(out data, bufferPosition);
         }
 
@@ -197,7 +233,7 @@ protected override void Read<T>(out T data, long bufferPosition = 0)
         /// <param name="bufferPosition">The offset within the buffer region of the shared memory to read from.</param>
         protected override void Read<T>(T[] buffer, long bufferPosition = 0)
         {
-            ReadWaitEvent.WaitOne();
+            ReadWait();
             base.Read<T>(buffer, bufferPosition);
         }
 
@@ -209,7 +245,7 @@ protected override void Read<T>(T[] buffer, long bufferPosition = 0)
         /// <param name="bufferPosition">The offset within the buffer region of the shared memory to read from.</param>
         protected override void Read(IntPtr destination, int length, long bufferPosition = 0)
         {
-            ReadWaitEvent.WaitOne();
+            ReadWait();
             base.Read(destination, length, bufferPosition);
         }
 
@@ -220,7 +256,7 @@ protected override void Read(IntPtr destination, int length, long bufferPosition
         /// <param name="bufferPosition">The offset within the buffer region of the shared memory to read from.</param>
         protected override void Read(Action<IntPtr> readFunc, long bufferPosition = 0)
         {
-            ReadWaitEvent.WaitOne();
+            ReadWait();
             base.Read(readFunc, bufferPosition);
         }
 

SharedMemory/SharedMemory.nuspec

@@ -2,7 +2,7 @@
 <package xmlns="http://schemas.microsoft.com/packaging/2012/06/nuspec.xsd">
     <metadata>
         <id>SharedMemory</id>
-        <version>$version$</version>
+        <version>2.0.16</version>
         <title>SharedMemory</title>
         <authors>Justin Stenning</authors>
         <owners>Justin Stenning</owners>
@@ -20,7 +20,10 @@ It features:
 
 Usage: http://sharedmemory.codeplex.com/documentation</description>
         <summary>Shared memory classes for sharing data between processes (Array, Buffer and Circular Buffer)</summary>
-        <releaseNotes>$version$
+        <releaseNotes>2.0.16
+1. Prevent possible deadlock situation with incorrect Acquire and Release of read/write locks in BufferWithLocks (affects BufferReadWrite and SharedArray).
+
+2.0.15
 1. Breaking change: Array, Buffer, and Header classes renamed to SharedArray, SharedBuffer, and SharedHeader
 2. Important breaking change! CircularBuffer Read/Write operations now allow an index to be specified. Check existing code as it may be passing a timeout value into index!
 3. Circular buffer keeps track of amount written to a node and uses this during read operations

SharedMemoryTests/BufferReadWriteTests.cs

@@ -38,5 +38,48 @@ public void ReadWrite_Bytes_DataMatches()
                 }
             }
         }
+
+        [TestMethod]
+        public void ReadWrite_TimeoutException()
+        {
+            bool timedout = false;
+            var name = Guid.NewGuid().ToString();
+            byte[] data = new byte[1024];
+            byte[] readData = new byte[1024];
+
+
+            using (var buf = new SharedMemory.BufferReadWrite(name, 1024))
+            using (var buf2 = new SharedMemory.BufferReadWrite(name))
+            {
+                // Set a small timeout to speed up the test
+                buf2.ReadWriteTimeout = 0;
+
+                // Introduce possible deadlock by acquiring without releasing the write lock.
+                buf.AcquireWriteLock();
+
+                // We want the AcquireReadLock to fail
+                if (!buf2.AcquireReadLock(1))
+                {
+                    try
+                    {
+                        // Read should timeout with TimeoutException because buf.ReleaseWriteLock has not been called
+                        buf2.Read(readData);
+                    }
+                    catch (TimeoutException e)
+                    {
+                        timedout = true;
+                    }
+                }
+                Assert.AreEqual(true, timedout, "The TimeoutException was not thrown.");
+
+                // Remove the deadlock situation, by releasing the write lock...
+                buf.ReleaseWriteLock();
+                // ...and ensure that we can now read the data
+                if (buf.AcquireReadLock(1))
+                    buf2.Read(readData);
+                else
+                    Assert.Fail("Failed to acquire read lock after releasing write lock.");
+            }
+        }
     }
 }