QPID-1116: fixed a race condition in connection/session close, session close now waits for the session to be detached before returning, this guarantees we won't have any active sessions when the connection close is attempted

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

7 files changed, 28 insertions, 10 deletions

diff --git a/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java b/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java
index bde60c433f..e741d4071c 100644
--- a/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java
+++ b/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java
@@ -152,7 +152,7 @@ public class AMQConnectionDelegate_0_10 implements AMQConnectionDelegate, Closed
     {
         if (_logger.isDebugEnabled())
         {
-            _logger.debug("Received a connection close from the broker: Error code : " + errorCode.getCode());
+            _logger.debug("Received a connection close from the broker: Error code : " + errorCode.getCode(), t);
         }
         if (_conn._exceptionListener != null)
         {
diff --git a/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java b/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java
index 17b7486fb7..e43072422c 100644
--- a/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java
+++ b/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java
@@ -210,8 +210,7 @@ public class AMQSession_0_10 extends AMQSession
     public void sendClose(long timeout) throws AMQException, FailoverException
     {
         getQpidSession().sync();
-        getQpidSession().sessionRequestTimeout(0);
-        getQpidSession().sessionDetach(getQpidSession().getName());
+        getQpidSession().close();
         getCurrentException();
     }
 
diff --git a/java/client/src/main/java/org/apache/qpidity/nclient/Client.java b/java/client/src/main/java/org/apache/qpidity/nclient/Client.java
index 5b45387be3..9fb4c541a9 100644
--- a/java/client/src/main/java/org/apache/qpidity/nclient/Client.java
+++ b/java/client/src/main/java/org/apache/qpidity/nclient/Client.java
@@ -21,6 +21,7 @@ package org.apache.qpidity.nclient;
 
 import java.util.List;
 import java.util.UUID;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.locks.Condition;
 import java.util.concurrent.locks.Lock;
@@ -266,7 +267,7 @@ public class Client implements org.apache.qpidity.nclient.Connection
                     closeOk.await(timeout - elapsed, TimeUnit.MILLISECONDS);
                     elapsed = System.currentTimeMillis() - start;
                 }
-                if(! closed )
+                if(!closed)
                 {
                     throw new QpidException("Timed out when closing connection", ErrorCode.CONNECTION_ERROR, null);
                 }
diff --git a/java/client/src/main/java/org/apache/qpidity/nclient/Session.java b/java/client/src/main/java/org/apache/qpidity/nclient/Session.java
index 833a26da87..c4d80e0fee 100644
--- a/java/client/src/main/java/org/apache/qpidity/nclient/Session.java
+++ b/java/client/src/main/java/org/apache/qpidity/nclient/Session.java
@@ -63,6 +63,8 @@ public interface Session
      */
     public void sync();
 
+    public void close();
+
     public void sessionDetach(byte[] name);
 
     public void sessionRequestTimeout(long expiry);
diff --git a/java/common/src/main/java/org/apache/qpidity/transport/ChannelDelegate.java b/java/common/src/main/java/org/apache/qpidity/transport/ChannelDelegate.java
index 5361613e1e..9470520937 100644
--- a/java/common/src/main/java/org/apache/qpidity/transport/ChannelDelegate.java
+++ b/java/common/src/main/java/org/apache/qpidity/transport/ChannelDelegate.java
@@ -42,9 +42,6 @@ class ChannelDelegate extends MethodDelegate<Channel>
     public @Override void sessionDetached(Channel channel, SessionDetached closed)
     {
         channel.getSession().closed();
-        // XXX: should we remove the channel from the connection? It
-        // could have an external reference to it. Maybe we need a
-        // weak hash map in connection.
     }
 
     public @Override void sessionDetach(Channel channel, SessionDetach dtc)
diff --git a/java/common/src/main/java/org/apache/qpidity/transport/Session.java b/java/common/src/main/java/org/apache/qpidity/transport/Session.java
index a7de66c1a7..c11ef46d36 100644
--- a/java/common/src/main/java/org/apache/qpidity/transport/Session.java
+++ b/java/common/src/main/java/org/apache/qpidity/transport/Session.java
@@ -459,7 +459,23 @@ public class Session extends Invoker
     {
         sessionRequestTimeout(0);
         sessionDetach(name);
-        // XXX: channel.close();
+        synchronized (commands)
+        {
+            long start = System.currentTimeMillis();
+            long elapsed = 0;
+            try
+            {
+                while (!closed.get() && elapsed < timeout)
+                {
+                    commands.wait(timeout - elapsed);
+                    elapsed = System.currentTimeMillis() - start;
+                }
+            }
+            catch (InterruptedException e)
+            {
+                throw new RuntimeException(e);
+            }
+        }
     }
 
     public void exception(Throwable t)
@@ -484,6 +500,9 @@ public class Session extends Invoker
                 }
             }
         }
+        channel.close();
+        channel.setSession(null);
+        channel = null;
     }
 
     public String toString()
diff --git a/java/log4j-test.xml b/java/log4j-test.xml
index d962069a86..c5fa8abe47 100644
--- a/java/log4j-test.xml
+++ b/java/log4j-test.xml
@@ -35,11 +35,11 @@
   </appender>
 
   <logger name="org.apache.qpid">
-    <level value="warn"/>
+    <level value="debug"/>
   </logger>
 
   <root>
-    <level value="warn"/>
+    <level value="debug"/>
     <appender-ref ref="console" />
   </root>
 </log4j:configuration>