Fix problems with sessions going out of scope and session numbers wrapping around.

Fixes QPID-1789: sessions that go out of scope without being detached will
detach themselves.

Also fixes several issues that arise when the session numbers wraps around
and start re-using old numbers.


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

3 files changed, 30 insertions, 10 deletions

diff --git a/cpp/src/qpid/client/ConnectionImpl.cpp b/cpp/src/qpid/client/ConnectionImpl.cpp
index c56d6a6807..6a46cb6249 100644
--- a/cpp/src/qpid/client/ConnectionImpl.cpp
+++ b/cpp/src/qpid/client/ConnectionImpl.cpp
@@ -95,11 +95,21 @@ ConnectionImpl::~ConnectionImpl() {
 void ConnectionImpl::addSession(const boost::shared_ptr<SessionImpl>& session, uint16_t channel)
 {
     Mutex::ScopedLock l(lock);
-    session->setChannel(channel == NEXT_CHANNEL ? nextChannel++ : channel);
-    boost::weak_ptr<SessionImpl>& s = sessions[session->getChannel()];
-    boost::shared_ptr<SessionImpl> ss = s.lock();
-    if (ss) throw SessionBusyException(QPID_MSG("Channel " << ss->getChannel() << " attached to " << ss->getId()));
-    s = session;
+    for (uint16_t i = 0; i < NEXT_CHANNEL; i++) { //will at most search through channels once
+        uint16_t c = channel == NEXT_CHANNEL ? nextChannel++ : channel;
+        boost::weak_ptr<SessionImpl>& s = sessions[c];
+        boost::shared_ptr<SessionImpl> ss = s.lock();
+        if (!ss) {
+            //channel is free, we can assign it to this session
+            session->setChannel(c);
+            s = session;
+            return;
+        } else if (channel != NEXT_CHANNEL) {
+            //channel is taken and was requested explicitly so don't look for another
+            throw SessionBusyException(QPID_MSG("Channel " << ss->getChannel() << " attached to " << ss->getId()));
+        } //else channel is busy, but we can keep looking for a free one
+    }
+
 }
 
 void ConnectionImpl::handle(framing::AMQFrame& frame)
@@ -165,7 +175,6 @@ void ConnectionImpl::open()
     } else {
         QPID_LOG(debug, "No security layer in place");
     }
-
     failover.reset(new FailoverListener(shared_from_this(), handler.knownBrokersUrls));
 }
 
@@ -246,7 +255,7 @@ const ConnectionSettings& ConnectionImpl::getNegotiatedSettings()
 {
     return handler;
 }
-    
+
 std::vector<qpid::Url> ConnectionImpl::getKnownBrokers() {
     return failover ? failover->getKnownBrokers() : handler.knownBrokersUrls;
 }
diff --git a/cpp/src/qpid/client/SessionImpl.cpp b/cpp/src/qpid/client/SessionImpl.cpp
index 32541dceac..d443a1170b 100644
--- a/cpp/src/qpid/client/SessionImpl.cpp
+++ b/cpp/src/qpid/client/SessionImpl.cpp
@@ -65,7 +65,8 @@ SessionImpl::SessionImpl(const std::string& name, boost::shared_ptr<ConnectionIm
       nextIn(0),
       nextOut(0),
       sendMsgCredit(0),
-      doClearDeliveryPropertiesExchange(true)
+      doClearDeliveryPropertiesExchange(true),
+      autoDetach(true)
 {
     channel.next = connectionShared.get();
 }
@@ -73,8 +74,11 @@ SessionImpl::SessionImpl(const std::string& name, boost::shared_ptr<ConnectionIm
 SessionImpl::~SessionImpl() {
     {
         Lock l(state);
-        if (state != DETACHED) {
-            QPID_LOG(warning, "Session was not closed cleanly");
+        if (state != DETACHED && state != DETACHING) {
+            QPID_LOG(warning, "Session was not closed cleanly: " << id);
+            // Inform broker but don't wait for detached as that deadlocks.
+            // The detached will be ignored as the channel will be invalid.
+            if (autoDetach) detach();
             setState(DETACHED);
             handleClosed();
             state.waitWaiters();
@@ -816,4 +820,6 @@ boost::shared_ptr<ConnectionImpl> SessionImpl::getConnection()
     return connectionWeak.lock();
 }
 
+void SessionImpl::disableAutoDetach() { autoDetach = false; }
+
 }}
diff --git a/cpp/src/qpid/client/SessionImpl.h b/cpp/src/qpid/client/SessionImpl.h
index 0624bb8b3c..cbd0742045 100644
--- a/cpp/src/qpid/client/SessionImpl.h
+++ b/cpp/src/qpid/client/SessionImpl.h
@@ -132,6 +132,9 @@ public:
 
     void setDoClearDeliveryPropertiesExchange(bool b=true) { doClearDeliveryPropertiesExchange = b; }
 
+    /** Suppress sending detach in destructor. Used by cluster to build session state */
+    void disableAutoDetach();
+
 private:
     enum State {
         INACTIVE,
@@ -247,6 +250,8 @@ private:
 
     bool doClearDeliveryPropertiesExchange;
 
+    bool autoDetach;
+    
   friend class client::SessionHandler;
 };