[Jboss-cvs] JBoss Messaging SVN: r1328 - in branches/Branch_1_0: src/main/org/jboss/messaging/core/local tests/bin

[jboss-cvs-commits at lists.jboss.org]

Author: ovidiu.feodorov at jboss.com
Date: 2006-09-20 17:10:48 -0400 (Wed, 20 Sep 2006)
New Revision: 1328

Modified:
   branches/Branch_1_0/src/main/org/jboss/messaging/core/local/FirstReceiverPointToPointRouter.java
   branches/Branch_1_0/src/main/org/jboss/messaging/core/local/SingleDestinationRouter.java
   branches/Branch_1_0/tests/bin/loop
Log:
fix for http://jira.jboss.org/jira/browse/JBMESSAGING-546 deadlock

Modified: branches/Branch_1_0/src/main/org/jboss/messaging/core/local/FirstReceiverPointToPointRouter.java
===================================================================
--- branches/Branch_1_0/src/main/org/jboss/messaging/core/local/FirstReceiverPointToPointRouter.java	2006-09-20 18:51:00 UTC (rev 1327)
+++ branches/Branch_1_0/src/main/org/jboss/messaging/core/local/FirstReceiverPointToPointRouter.java	2006-09-20 21:10:48 UTC (rev 1328)
@@ -26,6 +26,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
+import java.util.Collections;
 
 import org.jboss.logging.Logger;
 import org.jboss.messaging.core.Delivery;
@@ -74,44 +75,56 @@
 
    public Set handle(DeliveryObserver observer, Routable routable, Transaction tx)
    {
+      ArrayList receiversCopy;
+
+      synchronized(receivers)
+      {
+         if (receivers.isEmpty())
+         {
+            return Collections.EMPTY_SET;
+         }
+
+         // try to release the lock as quickly as possible and make a copy of the receivers array
+         // to avoid deadlock
+
+         receiversCopy = new ArrayList(receivers.size());
+         receiversCopy.addAll(receivers);
+      }
+
       Set deliveries = new HashSet();
-      
       boolean selectorRejected = false;
-      
-      synchronized(receivers)
+
+      for(Iterator i = receiversCopy.iterator(); i.hasNext(); )
       {
-         for(Iterator i = receivers.iterator(); i.hasNext(); )
+         Receiver receiver = (Receiver)i.next();
+
+         try
          {
-            Receiver receiver = (Receiver)i.next();
-            
-            try
+            Delivery d = receiver.handle(observer, routable, tx);
+
+            if (trace) { log.trace("receiver " + receiver + " handled " + routable + " and returned " + d); }
+
+            if (d != null && !d.isCancelled())
             {
-               Delivery d = receiver.handle(observer, routable, tx);
-
-               if (trace) { log.trace("receiver " + receiver + " handled " + routable + " and returned " + d); }
-     
-               if (d != null && !d.isCancelled())
+               if (d.isSelectorAccepted())
                {
-                  if (d.isSelectorAccepted())
-                  {
-                     // deliver to the first receiver that accepts
-                     deliveries.add(d);
-                     break;
-                  }
-                  else
-                  {
-                     selectorRejected = true;
-                  }
+                  // deliver to the first receiver that accepts
+                  deliveries.add(d);
+                  break;
                }
+               else
+               {
+                  selectorRejected = true;
+               }
             }
-            catch(Throwable t)
-            {
-               // broken receiver - log the exception and ignore it
-               log.error("The receiver " + receiver + " is broken", t);
-            }
          }
+         catch(Throwable t)
+         {
+            // broken receiver - log the exception and ignore it
+            log.error("The receiver " + receiver + " is broken", t);
+         }
       }
-      
+
       if (deliveries.isEmpty() && selectorRejected)
       {
          deliveries.add(new SimpleDelivery(null, null, true, false));

Modified: branches/Branch_1_0/src/main/org/jboss/messaging/core/local/SingleDestinationRouter.java
===================================================================
--- branches/Branch_1_0/src/main/org/jboss/messaging/core/local/SingleDestinationRouter.java	2006-09-20 18:51:00 UTC (rev 1327)
+++ branches/Branch_1_0/src/main/org/jboss/messaging/core/local/SingleDestinationRouter.java	2006-09-20 21:10:48 UTC (rev 1328)
@@ -58,19 +58,26 @@
 
    // Router implementation -----------------------------------------
 
-   public synchronized Set handle(DeliveryObserver observer, Routable routable, Transaction tx)
+   public Set handle(DeliveryObserver observer, Routable routable, Transaction tx)
    {
-      
-      
-      if (receiver == null)
+      Receiver receiverCopy;
+
+      synchronized(this)
       {
-         return Collections.EMPTY_SET;
+         if (receiver == null)
+         {
+            return Collections.EMPTY_SET;
+         }
+
+         // try to release the lock as quickly as possible and make a local copy of the receiver's
+         // reference, to avoid deadlock (http://jira.jboss.org/jira/browse/JBMESSAGING-546)
+         receiverCopy = receiver;
       }
 
       Set deliveries = new HashSet(1);
       try
       {
-         Delivery d = receiver.handle(observer, routable, tx);
+         Delivery d = receiverCopy.handle(observer, routable, tx);
          
          if (d != null && !d.isCancelled())
          {
@@ -80,7 +87,7 @@
       catch(Throwable t)
       {
          // broken receiver - log the exception and ignore it
-         log.error("The receiver " + receiver + " is broken" + t);
+         log.error("The receiver " + receiverCopy + " is broken" + t);
       }
       return deliveries;
    }

Modified: branches/Branch_1_0/tests/bin/loop
===================================================================
--- branches/Branch_1_0/tests/bin/loop	2006-09-20 18:51:00 UTC (rev 1327)
+++ branches/Branch_1_0/tests/bin/loop	2006-09-20 21:10:48 UTC (rev 1328)
@@ -1,7 +1,7 @@
 #!/bin/sh
 
 
-count=500
+count=5000
 i=0
 
 while [ $i -lt $count ]; do