https://issues.jboss.org/browse/HORNETQ-746 - Fixing a deadlock with …

…Netty NIO
hornetq · Aug 11, 2011 · d44fc02 · d44fc02
1 parent 834803d
commit d44fc02
Show file tree

Hide file tree

Showing 3 changed files with 359 additions and 54 deletions.
diff --git a/src/main/org/hornetq/core/server/impl/ServerConsumerImpl.java b/src/main/org/hornetq/core/server/impl/ServerConsumerImpl.java
@@ -93,8 +93,6 @@ private static void trace(final String message)
 
    private volatile LargeMessageDeliverer largeMessageDeliverer = null;
 
-   private boolean largeMessageInDelivery;
-
    /**
     * if we are a browse only consumer we don't need to worry about acknowledgemenets or being started/stopeed by the session.
     */
@@ -235,7 +233,7 @@ public HandleStatus handle(final MessageReference ref) throws Exception
 
          // If there is a pendingLargeMessage we can't take another message
          // This has to be checked inside the lock as the set to null is done inside the lock
-         if (largeMessageInDelivery)
+         if (largeMessageDeliverer != null)
          {
             return HandleStatus.BUSY;
          }
@@ -463,21 +461,6 @@ public void setTransferring(final boolean transferring)
       synchronized (lock)
       {
          this.transferring = transferring;
-
-         if (transferring)
-         {
-            // Now we must wait for any large message delivery to finish
-            while (largeMessageInDelivery)
-            {
-               try
-               {
-                  Thread.sleep(1);
-               }
-               catch (InterruptedException ignore)
-               {
-               }
-            }
-         }
       }
 
       // Outside the lock
@@ -662,25 +645,27 @@ public AtomicInteger getAvailableCredits()
 
    private void promptDelivery()
    {
-      synchronized (lock)
+      // largeMessageDeliverer is aways set inside a lock
+      // if we don't acquire a lock, we will have NPE eventually
+      if (largeMessageDeliverer != null)
       {
-         // largeMessageDeliverer is aways set inside a lock
-         // if we don't acquire a lock, we will have NPE eventually
-         if (largeMessageDeliverer != null)
-         {
-            resumeLargeMessage();
-         }
-         else
-         {
-            if (browseOnly)
-            {
-               messageQueue.getExecutor().execute(browserDeliverer);
-            }
-            else
-            {
-               messageQueue.forceDelivery();
-            }
-         }
+         resumeLargeMessage();
+      }
+      else
+      {
+         forceDelivery();
+      }
+   }
+
+   private void forceDelivery()
+   {
+      if (browseOnly)
+      {
+         messageQueue.getExecutor().execute(browserDeliverer);
+      }
+      else
+      {
+         messageQueue.deliverAsync();
       }
    }
 
@@ -691,8 +676,6 @@ private void resumeLargeMessage()
 
    private void deliverLargeMessage(final MessageReference ref, final ServerMessage message) throws Exception
    {
-      largeMessageInDelivery = true;
-
       final LargeMessageDeliverer localDeliverer = new LargeMessageDeliverer((LargeServerMessage)message, ref);
 
       // it doesn't need lock because deliverLargeMesasge is already inside the lock()
@@ -714,6 +697,7 @@ private void deliverStandardMessage(final MessageReference ref, final ServerMess
       }
    }
 
+
    // Inner classes
    // ------------------------------------------------------------------------
 
@@ -727,16 +711,7 @@ public void run()
             {
                if (largeMessageDeliverer == null || largeMessageDeliverer.deliver())
                {
-                  if (browseOnly)
-                  {
-                     messageQueue.getExecutor().execute(browserDeliverer);
-                  }
-                  else
-                  {
-                     // prompt Delivery only if chunk was finished
-
-                     messageQueue.deliverAsync();
-                  }
+                  forceDelivery();
                }
             }
             catch (Exception e)
@@ -901,8 +876,6 @@ public void finish() throws Exception
 
             largeMessageDeliverer = null;
 
-            largeMessageInDelivery = false;
-
             largeMessage = null;
          }
       }

diff --git a/src/main/org/hornetq/utils/ConfigurationHelper.java b/src/main/org/hornetq/utils/ConfigurationHelper.java
@@ -73,7 +73,7 @@ public static int getIntProperty(final String propName, final int def, final Map
          {
             return Integer.valueOf((String)prop);
          }
-         else if (prop instanceof Integer == false)
+         else if (prop instanceof Number == false)
          {
             ConfigurationHelper.log.warn("Property " + propName +
                                          " must be an Integer, it is " +
@@ -83,7 +83,7 @@ else if (prop instanceof Integer == false)
          }
          else
          {
-            return (Integer)prop;
+            return ((Number)prop).intValue();
          }
       }
    }
@@ -108,7 +108,7 @@ public static long getLongProperty(final String propName, final long def, final
          {
             return Long.valueOf((String)prop);
          }
-         else if (prop instanceof Long == false)
+         else if (prop instanceof Number == false)
          {
             ConfigurationHelper.log.warn("Property " + propName +
                                          " must be an Long, it is " +
@@ -118,7 +118,7 @@ else if (prop instanceof Long == false)
          }
          else
          {
-            return (Long)prop;
+            return ((Number)prop).longValue();
          }
       }
    }