QPID-1809, QPID-2081 : Corrected ChannelClose logic. Removed an unnecessary sync on the failoverMutex in AMQSession that was causing the notification of the close to be blocked until a TimeOutException occured.

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@812153 13f79535-47bb-0310-9956-ffa450edef68

1 files changed, 48 insertions, 15 deletions

diff --git a/java/systests/src/main/java/org/apache/qpid/test/unit/close/JavaServerCloseRaceConditionTest.java b/java/systests/src/main/java/org/apache/qpid/test/unit/close/JavaServerCloseRaceConditionTest.java
index 3eddc8d7a5..3fb6cd3526 100644
--- a/java/systests/src/main/java/org/apache/qpid/test/unit/close/JavaServerCloseRaceConditionTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/test/unit/close/JavaServerCloseRaceConditionTest.java
@@ -20,7 +20,6 @@
  */
 package org.apache.qpid.test.unit.close;
 
-import org.apache.qpid.client.AMQAuthenticationException;
 import org.apache.qpid.client.AMQConnection;
 import org.apache.qpid.client.AMQDestination;
 import org.apache.qpid.client.AMQSession;
@@ -32,9 +31,52 @@ import org.apache.qpid.test.utils.QpidTestCase;
 
 import javax.jms.Session;
 
-/** QPID-1085 */
+/** QPID-1809
+ *
+ * Race condition on error handling and close logic.
+ *
+ * See most often with SimpleACLTest as this test is the expects the server to
+ * shut the connection/channels. This sort of testing is not performed by many,
+ * if any, of the other system tests.
+ *
+ * The problem is that we have two threads
+ *
+ *  MainThread              Exception(Mina)Thread
+ *     |                          |
+ *    Performs                    |
+ *     ACtion                     |
+ *     |                      Receives Server
+ *     |                        Close
+ *  Blocks for                    |
+ *    Response                    |
+ *      |                     Starts To Notify
+ *      |                        client
+ *      |                         |
+ *      |             <----- Notify Main Thread
+ *    Notification                |
+ *     wakes client               |
+ *      |                         |
+ *     Client then                |
+ * processes Error.               |
+ *      |                         |
+ *   Potentially Attempting      Close Channel/Connection
+ *      Connection Close
+ *
+ * The two threads both attempt to close the connection but the main thread does
+ * so assuming that the connection is open and valid.
+ *
+ * The Exception thread must modify the connection so that no furter syncWait
+ * commands are performed.
+ *
+ * This test sends an ExchangeDeclare that is Asynchronous and will fail and
+ * so cause a ChannelClose error but we perform a syncWait so that we can be
+ * sure to test that the BlockingWaiter is correctly awoken.
+ *  
+ */
 public class JavaServerCloseRaceConditionTest extends QpidTestCase
 {
+    private static final String EXCHANGE_NAME = "NewExchangeNametoFailLookup";
+
     public void test() throws Exception
     {
 
@@ -42,14 +84,11 @@ public class JavaServerCloseRaceConditionTest extends QpidTestCase
 
         AMQSession session = (AMQSession) connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
 
-        AMQDestination destination = (AMQDestination) session.createQueue(getTestQueueName());
-
         // Set no wait true so that we block the connection
         // Also set a different exchange class string so the attempt to declare
         // the exchange causes an exchange. 
-        ExchangeDeclareBody body = session.getMethodRegistry().createExchangeDeclareBody(session.getTicket(), destination.getExchangeName(), new AMQShortString("NewTypeForException"),
-                                                                                         destination.getExchangeName().toString().startsWith("amq."),
-                                                                                         false, false, false, true, null);
+        ExchangeDeclareBody body = session.getMethodRegistry().createExchangeDeclareBody(session.getTicket(), new AMQShortString(EXCHANGE_NAME), null,
+                                                                                         true, false, false, false, true, null);
 
         AMQFrame exchangeDeclare = body.generateFrame(session.getChannelId());
 
@@ -60,10 +99,7 @@ public class JavaServerCloseRaceConditionTest extends QpidTestCase
         }
         catch (Exception e)
         {
-            if (!(e instanceof AMQAuthenticationException))
-            {
-                fail("Cause was not AMQAuthenticationException. Was " + e.getClass() + ":" + e.getMessage());
-            }
+            assertTrue("Exception should say the exchange is not known.", e.getMessage().contains("Unknown exchange: " + EXCHANGE_NAME));
         }
 
         try
@@ -76,10 +112,7 @@ public class JavaServerCloseRaceConditionTest extends QpidTestCase
         }
         catch (Exception e)
         {
-            if (!(e instanceof AMQAuthenticationException))
-            {
-                fail("Cause was not AMQAuthenticationException. Was " + e.getClass() + ":" + e.getMessage());
-            }
+            assertTrue("Exception should say the exchange is not known.", e.getMessage().contains("Unknown exchange: " + EXCHANGE_NAME));
         }
 
     }