Make Session::close and Connection::close no-throw

close() will often be called in destructors and so should not throw exceptions.


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

4 files changed, 46 insertions, 10 deletions

diff --git a/qpid/cpp/src/qpid/client/ConnectionImpl.cpp b/qpid/cpp/src/qpid/client/ConnectionImpl.cpp
index c48a580fe8..e4e7b2f5c2 100644
--- a/qpid/cpp/src/qpid/client/ConnectionImpl.cpp
+++ b/qpid/cpp/src/qpid/client/ConnectionImpl.cpp
@@ -188,13 +188,16 @@ void ConnectionImpl::idleOut()
 
 void ConnectionImpl::close()
 {
-    if (heartbeatTask) {
+    if (heartbeatTask) 
         heartbeatTask->cancel();
+    // close() must be idempotent and no-throw as it will often be called in destructors.
+    if (handler.isOpen()) {
+        try {
+            handler.close();
+            closed(CLOSE_CODE_NORMAL, "Closed by client");
+        } catch (...) {}
     }
-
-    if (!handler.isOpen()) return;
-    handler.close();
-    closed(CLOSE_CODE_NORMAL, "Closed by client");
+    assert(!handler.isOpen());
 }
 
 
diff --git a/qpid/cpp/src/qpid/client/SessionImpl.cpp b/qpid/cpp/src/qpid/client/SessionImpl.cpp
index 7c807558f0..0f767c9f2e 100644
--- a/qpid/cpp/src/qpid/client/SessionImpl.cpp
+++ b/qpid/cpp/src/qpid/client/SessionImpl.cpp
@@ -119,10 +119,15 @@ void SessionImpl::open(uint32_t timeout) // user thread
 void SessionImpl::close() //user thread
 {
     Lock l(state);
-    if (state == DETACHED || state == DETACHING) return;
-    if (detachedLifetime) setTimeout(0);
-    detach();
-    waitFor(DETACHED);
+    // close() must be idempotent and no-throw as it will often be called in destructors.
+    if (state != DETACHED && state != DETACHING) {
+        try {
+            if (detachedLifetime) setTimeout(0);
+            detach();
+            waitFor(DETACHED);
+        } catch (...) {}
+        setState(DETACHED);
+    }
 }
 
 void SessionImpl::resume(boost::shared_ptr<ConnectionImpl>) // user thread
diff --git a/qpid/cpp/src/tests/BrokerFixture.h b/qpid/cpp/src/tests/BrokerFixture.h
index f56a925b81..5eb6858491 100644
--- a/qpid/cpp/src/tests/BrokerFixture.h
+++ b/qpid/cpp/src/tests/BrokerFixture.h
@@ -127,7 +127,7 @@ struct ClientT {
         : connection(settings), session(connection.newSession(name_)), subs(session), name(name_) {}
 
     ~ClientT() { close(); }
-    void close() { if (connection.isOpen()) { session.close(); connection.close(); } }
+    void close() { session.close(); connection.close(); }
 };
 
 typedef ClientT<> Client;
diff --git a/qpid/cpp/src/tests/PartialFailure.cpp b/qpid/cpp/src/tests/PartialFailure.cpp
index 5de8ecb189..b5772607b2 100644
--- a/qpid/cpp/src/tests/PartialFailure.cpp
+++ b/qpid/cpp/src/tests/PartialFailure.cpp
@@ -102,6 +102,10 @@ QPID_AUTO_TEST_CASE(testCoincidentErrors) {
         try { Client c11(cluster[1], "c11"); ++alive; } catch (...) {}
 
         BOOST_CHECK_EQUAL(alive, 1);
+
+        // Close inside ScopedSuppressLogging to avoid warnings 
+        c0.close();
+        c1.close();
     }
 }
 
@@ -138,6 +142,11 @@ QPID_AUTO_TEST_CASE(testNormalErrors) {
         BOOST_CHECK_EQUAL(3u, knownBrokerPorts(c2.connection, 3).size());
         BOOST_CHECK_EQUAL(c2.subs.get("c0", TIMEOUT).getData(), "stay");
         BOOST_CHECK_EQUAL(c2.subs.get("c1", TIMEOUT).getData(), "stay");
+
+        // Close inside ScopedSuppressLogging to avoid warnings 
+        c0.close();
+        c1.close();
+        c2.close();
     }
 }
 
@@ -166,6 +175,11 @@ QPID_AUTO_TEST_CASE(testErrorAfterJoin) {
         BOOST_CHECK_THROW(c0.session.messageTransfer(content=pMessage("xxx", "q")), TransportFailure);
 
         BOOST_CHECK_EQUAL(1u, knownBrokerPorts(c2.connection, 1).size());
+
+        // Close inside ScopedSuppressLogging to avoid warnings 
+        c0.close();
+        c1.close();
+        c2.close();
     }
 }
 
@@ -196,6 +210,11 @@ QPID_AUTO_TEST_CASE(testSinglePartialFailure) {
         c0.session.messageTransfer(content=pMessage("c", "q"));
         BOOST_CHECK_EQUAL(c0.session.queueQuery("q").getMessageCount(), 5u);
         BOOST_CHECK_EQUAL(1u, knownBrokerPorts(c0.connection, 1).size());
+
+        // Close inside ScopedSuppressLogging to avoid warnings 
+        c0.close();
+        c1.close();
+        c2.close();
     }
 }
 
@@ -224,6 +243,12 @@ QPID_AUTO_TEST_CASE(testMultiPartialFailure) {
         // FIXME aconway 2009-06-30: This check fails sporadically with 2 != 3.
         // It should pass reliably.
         // BOOST_CHECK_EQUAL(2u, knownBrokerPorts(c0.connection, 2).size());
+
+        // Close inside ScopedSuppressLogging to avoid warnings 
+        c0.close();
+        c1.close();
+        c2.close();
+        c3.close();
     }
 }
 
@@ -254,6 +279,9 @@ QPID_AUTO_TEST_CASE(testPartialFailureMemberLeaves) {
         Client c00(cluster[0], "c00"); // Old connection is dead.
         BOOST_CHECK_EQUAL(c00.session.queueQuery("q").getMessageCount(), 1u);
         BOOST_CHECK_EQUAL(1u, knownBrokerPorts(c00.connection, 1).size());
+
+        // Close inside ScopedSuppressLogging to avoid warnings 
+        c0.close();
     }
 }
 #endif