You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by ta...@apache.org on 2014/08/12 19:44:50 UTC

svn commit: r1617540 - in /activemq/activemq-dotnet/Apache.NMS.ActiveMQ/branches/1.6.x: ./ src/main/csharp/Threads/CompositeTaskRunner.cs src/test/csharp/Threads/CompositeTaskRunnerTest.cs

Author: tabish
Date: Tue Aug 12 17:44:50 2014
New Revision: 1617540

URL: http://svn.apache.org/r1617540
Log:
https://issues.apache.org/jira/browse/AMQNET-488

apply fix for potential deadlock on failover.

Modified:
    activemq/activemq-dotnet/Apache.NMS.ActiveMQ/branches/1.6.x/   (props changed)
    activemq/activemq-dotnet/Apache.NMS.ActiveMQ/branches/1.6.x/src/main/csharp/Threads/CompositeTaskRunner.cs
    activemq/activemq-dotnet/Apache.NMS.ActiveMQ/branches/1.6.x/src/test/csharp/Threads/CompositeTaskRunnerTest.cs

Propchange: activemq/activemq-dotnet/Apache.NMS.ActiveMQ/branches/1.6.x/
------------------------------------------------------------------------------
  Merged /activemq/activemq-dotnet/Apache.NMS.ActiveMQ/trunk:r1617525

Modified: activemq/activemq-dotnet/Apache.NMS.ActiveMQ/branches/1.6.x/src/main/csharp/Threads/CompositeTaskRunner.cs
URL: http://svn.apache.org/viewvc/activemq/activemq-dotnet/Apache.NMS.ActiveMQ/branches/1.6.x/src/main/csharp/Threads/CompositeTaskRunner.cs?rev=1617540&r1=1617539&r2=1617540&view=diff
==============================================================================
--- activemq/activemq-dotnet/Apache.NMS.ActiveMQ/branches/1.6.x/src/main/csharp/Threads/CompositeTaskRunner.cs (original)
+++ activemq/activemq-dotnet/Apache.NMS.ActiveMQ/branches/1.6.x/src/main/csharp/Threads/CompositeTaskRunner.cs Tue Aug 12 17:44:50 2014
@@ -17,6 +17,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Threading;
 
 namespace Apache.NMS.ActiveMQ.Threads
@@ -185,21 +186,28 @@ namespace Apache.NMS.ActiveMQ.Threads
 		
 		private bool Iterate()
 		{
-			lock(mutex)
+		    Task pendingTask = null;
+
+            lock (mutex)
+		    {                
+                foreach (CompositeTask task in this.tasks)
+			    {            
+                    if (task.IsPending)
+                    {
+                        pendingTask = task;
+                        break;
+                    }
+                }
+            }
+
+            if (pendingTask != null)
 			{
-				foreach(CompositeTask task in this.tasks)
-				{
-					if(task.IsPending)
-					{
-						task.Iterate();
-
-						// Always return true here so that we can check the next
-						// task in the list to see if its done.
-						return true;
-					}
-				}
+                pendingTask.Iterate();
+				// Always return true here so that we can check the next
+			    // task in the list to see if its done.
+				return true;
 			}
-			
+
 			return false;
 		}
     }

Modified: activemq/activemq-dotnet/Apache.NMS.ActiveMQ/branches/1.6.x/src/test/csharp/Threads/CompositeTaskRunnerTest.cs
URL: http://svn.apache.org/viewvc/activemq/activemq-dotnet/Apache.NMS.ActiveMQ/branches/1.6.x/src/test/csharp/Threads/CompositeTaskRunnerTest.cs?rev=1617540&r1=1617539&r2=1617540&view=diff
==============================================================================
--- activemq/activemq-dotnet/Apache.NMS.ActiveMQ/branches/1.6.x/src/test/csharp/Threads/CompositeTaskRunnerTest.cs (original)
+++ activemq/activemq-dotnet/Apache.NMS.ActiveMQ/branches/1.6.x/src/test/csharp/Threads/CompositeTaskRunnerTest.cs Tue Aug 12 17:44:50 2014
@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+using System;
 using System.Threading;
 using Apache.NMS.ActiveMQ.Threads;
 using NUnit.Framework;
@@ -24,7 +25,6 @@ namespace Apache.NMS.ActiveMQ.Test.Threa
     [TestFixture]
     public class CompositeTaskRunnerTest
     {
-
         class CountingTask : CompositeTask
         {
             private int count;
@@ -90,5 +90,59 @@ namespace Apache.NMS.ActiveMQ.Test.Threa
             runner.RemoveTask(task1);
             runner.RemoveTask(task2);
         }
+
+        [Test]
+        public void CompositeTaskRunnerDoesntHoldLockWhileCallingIterate()
+        {
+            object lockObj = new object();
+            
+            // Start a task running that takes a shared lock during it's iterate.
+            CompositeTaskRunner runner = new CompositeTaskRunner();
+            runner.AddTask(new LockingTask(lockObj));
+           
+            // Start a separate thread that holds that same lock whilst manipulating the CompositeTaskRunner (See InactivityMonitor for real example).
+            AutoResetEvent resetEvent = new AutoResetEvent(false);
+
+            ThreadPool.QueueUserWorkItem(_ =>
+            {
+                for (int i = 0; i < 10000; i++)
+                {
+                    lock (lockObj)
+                    {
+                        var countingTask = new CountingTask("task1", 100);
+                        runner.AddTask(countingTask);
+                        runner.RemoveTask(countingTask);
+                    }
+                }
+
+                resetEvent.Set();
+            });
+
+            // Wait for the second thread to finish 10000 attempts.
+            Assert.That(resetEvent.WaitOne(TimeSpan.FromSeconds(10)), "The secondary lock user didn't finish 10000 iterations in less than 10 seconds. Probably dead locked!");
+            runner.Shutdown();
+        }
+
+        private class LockingTask : CompositeTask
+        {
+            private readonly object _lockObj;
+
+            public LockingTask(object lockObj)
+            {
+                _lockObj = lockObj;
+                IsPending = true;
+            }
+
+            public bool Iterate()
+            {
+                lock (_lockObj)
+                {
+                    Thread.Sleep(10);
+                }
+                return true;
+            }
+
+            public bool IsPending { get; private set; }
+        }
     }
-}
+}
\ No newline at end of file