[jboss-cvs] JBoss Messaging SVN: r8151 - in branches/Branch_1_4: tests/src/org/jboss/test/messaging/jms and 1 other directory.

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

Author: gaohoward
Date: 2010-12-16 07:22:22 -0500 (Thu, 16 Dec 2010)
New Revision: 8151

Modified:
   branches/Branch_1_4/src/main/org/jboss/jms/client/remoting/ConsolidatedRemotingConnectionListener.java
   branches/Branch_1_4/tests/src/org/jboss/test/messaging/jms/ConnectionTest.java
Log:
JBMESSAGING-1825


Modified: branches/Branch_1_4/src/main/org/jboss/jms/client/remoting/ConsolidatedRemotingConnectionListener.java
===================================================================
--- branches/Branch_1_4/src/main/org/jboss/jms/client/remoting/ConsolidatedRemotingConnectionListener.java	2010-12-10 08:49:15 UTC (rev 8150)
+++ branches/Branch_1_4/src/main/org/jboss/jms/client/remoting/ConsolidatedRemotingConnectionListener.java	2010-12-16 12:22:22 UTC (rev 8151)
@@ -47,6 +47,8 @@
 
    private JMSException jmsException;
 
+   private Object jmsExceptionLock = new Object();
+
    // Constructors ---------------------------------------------------------------------------------
 
    public ConsolidatedRemotingConnectionListener()
@@ -86,33 +88,36 @@
       
       if (forwardToJMSListener)
       {
-         //the connection is broken, we need to remember this
-         if (throwable instanceof Error)
+         // if a listener already set, notify it instantly and forget the exception.
+         synchronized (jmsExceptionLock)
          {
-            final String msg = "Caught Error on underlying remoting connection";
-            log.error(this + ": " + msg, throwable);
-            jmsException = new JMSException(msg + ": " + throwable.getMessage());
+            // the connection is broken, we need to remember this
+            if (throwable instanceof Error)
+            {
+               final String msg = "Caught Error on underlying remoting connection";
+               log.error(this + ": " + msg, throwable);
+               jmsException = new JMSException(msg + ": " + throwable.getMessage());
+            }
+            else if (throwable instanceof Exception)
+            {
+               Exception e = (Exception)throwable;
+               jmsException = new JMSException("Failure on underlying remoting connection");
+               jmsException.setLinkedException(e);
+            }
+            else
+            {
+               // Some other Throwable subclass
+               final String msg = "Caught Throwable on underlying remoting connection";
+               log.error(this + ": " + msg, throwable);
+               jmsException = new JMSException(msg + ": " + throwable.getMessage());
+            }
+            
+            if (jmsExceptionListener != null)
+            {
+               jmsExceptionListener.onException(jmsException);
+               jmsException = null;
+            }
          }
-         else if (throwable instanceof Exception)
-         {
-            Exception e = (Exception)throwable;
-            jmsException = new JMSException("Failure on underlying remoting connection");
-            jmsException.setLinkedException(e);
-         }
-         else
-         {
-            // Some other Throwable subclass
-            final String msg = "Caught Throwable on underlying remoting connection";
-            log.error(this + ": " + msg, throwable);
-            jmsException = new JMSException(msg + ": " + throwable.getMessage());
-         }
-         
-         //if a listener already set, notify it instantly and forget the exception.
-         if (jmsExceptionListener != null)
-         {
-            jmsExceptionListener.onException(jmsException);
-            jmsException = null;
-         }
       }
    }
 
@@ -133,16 +138,19 @@
    public synchronized void addJMSExceptionListener(ExceptionListener listener)
    {
       log.trace(this + " adding JMS exception listener " + listener + " current exception: " + jmsException);
+
+      synchronized (jmsExceptionLock)
+      {
+         this.jmsExceptionListener = listener;
       
-      this.jmsExceptionListener = listener;
-      
-      //if a connection failure event already happened, we notify it.
-      //https://jira.jboss.org/jira/browse/JBMESSAGING-1776
-      if (jmsException != null)
-      {
-         final JMSException copy = jmsException;
-         jmsException = null;
-         notifyJMSExceptionListener(listener, copy);
+         // if a connection failure event already happened, we notify it.
+         // https://jira.jboss.org/jira/browse/JBMESSAGING-1776
+         if (jmsException != null)
+         {
+            final JMSException copy = jmsException;
+            jmsException = null;
+            notifyJMSExceptionListener(listener, copy);
+         }
       }
    }
    
@@ -158,9 +166,12 @@
       }.start();
    }
 
-   public synchronized ExceptionListener getJMSExceptionListener()
+   public ExceptionListener getJMSExceptionListener()
    {
-      return jmsExceptionListener;
+      synchronized (jmsExceptionLock)
+      {
+         return jmsExceptionListener;
+      }
    }
 
    /**
@@ -168,11 +179,19 @@
     */
    public synchronized void clear()
    {
-      jmsExceptionListener = null;
+      clearJmsExceptionListener();
       remotingListener = null;
       log.trace(this + " cleared");
    }
 
+   private void clearJmsExceptionListener()
+   {
+      synchronized (jmsExceptionLock)
+      {
+         jmsExceptionListener = null;
+      }
+   }
+
    public void setConnectionState(ConnectionState state)
    {
       this.state = state;

Modified: branches/Branch_1_4/tests/src/org/jboss/test/messaging/jms/ConnectionTest.java
===================================================================
--- branches/Branch_1_4/tests/src/org/jboss/test/messaging/jms/ConnectionTest.java	2010-12-10 08:49:15 UTC (rev 8150)
+++ branches/Branch_1_4/tests/src/org/jboss/test/messaging/jms/ConnectionTest.java	2010-12-16 12:22:22 UTC (rev 8151)
@@ -35,6 +35,7 @@
 
 import org.jboss.jms.client.JBossConnection;
 import org.jboss.jms.client.delegate.ClientConnectionDelegate;
+import org.jboss.jms.client.remoting.ConsolidatedRemotingConnectionListener;
 import org.jboss.jms.client.state.ConnectionState;
 import org.jboss.jms.tx.ResourceManager;
 import org.jboss.jms.tx.ResourceManagerFactory;
@@ -429,6 +430,62 @@
          }
       }
    }
+
+   public void testExceptionListener3() throws Exception
+   {
+      ConsolidatedRemotingConnectionListener jbmListener = new ConsolidatedRemotingConnectionListener();
+      jbmListener.start();
+      jbmListener.handleConnectionException(new Exception("bad connection"), null);
+      MyJMSListener jmsListener = new MyJMSListener();
+      jbmListener.addJMSExceptionListener(jmsListener);
+      
+      //because notifying the listener is from a separate thread
+      try
+      {
+         Thread.sleep(5000);
+      }
+      catch (InterruptedException e)
+      {
+         //ignore
+      }
+      
+      if (jmsListener.getNum() == 0)
+      {
+         //if 5 sec still not fired, we add another 5 sec
+         try
+         {
+            Thread.sleep(5000);
+         }
+         catch (InterruptedException e)
+         {
+            //ignore
+         }
+      }
+      assertEquals(1, jmsListener.getNum());
+      assertEquals("Failure on underlying remoting connection", jmsListener.getErrorMessage());
+   }
+
+   public static class MyJMSListener implements ExceptionListener
+   {
+      int num = 0;
+      String errMsg = null;
+      
+      public synchronized void onException(JMSException e)
+      {
+         num++;
+         errMsg = e.getMessage();
+      }
+      
+      public synchronized String getErrorMessage()
+      {
+         return errMsg;
+      }
+      
+      public synchronized int getNum()
+      {
+         return num;
+      }
+   }
    
    /*
     * See http://jira.jboss.com/jira/browse/JBMESSAGING-635