QPID-193 Make createConsumer synchronous to avoid races in client code.

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

2 files changed, 83 insertions, 9 deletions

diff --git a/java/client/src/main/java/org/apache/qpid/client/AMQSession.java b/java/client/src/main/java/org/apache/qpid/client/AMQSession.java
index 8e93b19eea..0cbf5bac60 100644
--- a/java/client/src/main/java/org/apache/qpid/client/AMQSession.java
+++ b/java/client/src/main/java/org/apache/qpid/client/AMQSession.java
@@ -870,7 +870,7 @@ public class AMQSession extends Closeable implements Session, QueueSession, Topi
 
                 try
                 {
-                    registerConsumer(consumer);
+                    registerConsumer(consumer, false);
                 }
                 catch (AMQException e)
                 {
@@ -948,7 +948,7 @@ public class AMQSession extends Closeable implements Session, QueueSession, Topi
      * @return the consumer tag generated by the broker
      */
     private String consumeFromQueue(String queueName, AMQProtocolHandler protocolHandler, int prefetchHigh, int prefetchLow,
-                                    boolean noLocal, boolean exclusive, int acknowledgeMode) throws AMQException
+                                    boolean noLocal, boolean exclusive, int acknowledgeMode, boolean nowait) throws AMQException
     {
         //fixme prefetch values are not used here. Do we need to have them as parametsrs?
         //need to generate a consumer tag on the client so we can exploit the nowait flag
@@ -957,9 +957,15 @@ public class AMQSession extends Closeable implements Session, QueueSession, Topi
         AMQFrame jmsConsume = BasicConsumeBody.createAMQFrame(_channelId, 0,
                                                               queueName, tag, noLocal,
                                                               acknowledgeMode == Session.NO_ACKNOWLEDGE,
-                                                              exclusive, true);
-
-        protocolHandler.writeFrame(jmsConsume);
+                                                              exclusive, nowait);
+        if (nowait)
+        {
+            protocolHandler.writeFrame(jmsConsume);
+        }
+        else
+        {
+            protocolHandler.syncWrite(jmsConsume, BasicConsumeOkBody.class);
+        }
         return tag;
     }
 
@@ -1263,7 +1269,7 @@ public class AMQSession extends Closeable implements Session, QueueSession, Topi
      * @param consumer
      * @throws AMQException
      */
-    void registerConsumer(BasicMessageConsumer consumer) throws AMQException
+    void registerConsumer(BasicMessageConsumer consumer, boolean nowait) throws AMQException
     {
         AMQDestination amqd = consumer.getDestination();
 
@@ -1275,8 +1281,9 @@ public class AMQSession extends Closeable implements Session, QueueSession, Topi
 
         bindQueue(amqd, queueName, protocolHandler, consumer.getRawSelectorFieldTable());
 
-        String consumerTag = consumeFromQueue(queueName, protocolHandler, consumer.getPrefetchHigh(), consumer.getPrefetchLow(),
-                                              consumer.isNoLocal(), consumer.isExclusive(), consumer.getAcknowledgeMode());
+        String consumerTag = consumeFromQueue(queueName, protocolHandler, consumer.getPrefetchHigh(),
+                                              consumer.getPrefetchLow(), consumer.isNoLocal(), consumer.isExclusive(),
+                                              consumer.getAcknowledgeMode(), nowait);
 
         consumer.setConsumerTag(consumerTag);
         _consumers.put(consumerTag, consumer);
@@ -1338,7 +1345,7 @@ public class AMQSession extends Closeable implements Session, QueueSession, Topi
         for (Iterator it = consumers.iterator(); it.hasNext();)
         {
             BasicMessageConsumer consumer = (BasicMessageConsumer) it.next();
-            registerConsumer(consumer);
+            registerConsumer(consumer, true);
         }
     }
 
diff --git a/java/client/src/test/java/org/apache/qpid/test/unit/basic/PubSubTwoConnectionTest.java b/java/client/src/test/java/org/apache/qpid/test/unit/basic/PubSubTwoConnectionTest.java
new file mode 100644
index 0000000000..f4814795c4
--- /dev/null
+++ b/java/client/src/test/java/org/apache/qpid/test/unit/basic/PubSubTwoConnectionTest.java
@@ -0,0 +1,67 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.qpid.test.unit.basic;
+
+import junit.framework.TestCase;
+import org.apache.qpid.client.transport.TransportConnection;
+import org.apache.qpid.client.AMQConnection;
+import org.apache.qpid.client.AMQSession;
+import org.apache.qpid.client.AMQTopic;
+
+import javax.jms.*;
+
+/**
+ * @author Apache Software Foundation
+ */
+public class PubSubTwoConnectionTest extends TestCase
+{
+    protected void setUp() throws Exception
+    {
+        super.setUp();
+        TransportConnection.createVMBroker(1);
+    }
+
+    protected void tearDown() throws Exception
+    {
+        super.tearDown();
+    }
+
+    /**
+     * This tests that a consumer is set up synchronously
+     * @throws Exception
+     */
+    public void testTwoConnections() throws Exception
+    {
+        Topic topic = new AMQTopic("MyTopic");
+        Connection con1 = new AMQConnection("vm://:1", "guest", "guest", "Client1", "/test_path");
+        Session session1 = con1.createSession(false, AMQSession.NO_ACKNOWLEDGE);
+        MessageProducer producer = session1.createProducer(topic);
+
+        Connection con2 = new AMQConnection("vm://:1", "guest", "guest", "Client2", "/test_path");
+        Session session2 = con2.createSession(false, AMQSession.NO_ACKNOWLEDGE);
+        MessageConsumer consumer = session2.createConsumer(topic);
+        con2.start();        
+        producer.send(session1.createTextMessage("Hello"));
+        TextMessage tm1 = (TextMessage) consumer.receive(2000);
+        assertNotNull(tm1);
+        assertEquals("Hello", tm1.getText());
+    }
+}