SessionImpl using ExceptionHolder to generate correctly typed exceptions..

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

5 files changed, 44 insertions, 49 deletions

diff --git a/cpp/rubygen/framing.0-10/constants.rb b/cpp/rubygen/framing.0-10/constants.rb
index c2b43dbf37..0560cef887 100755
--- a/cpp/rubygen/framing.0-10/constants.rb
+++ b/cpp/rubygen/framing.0-10/constants.rb
@@ -93,9 +93,9 @@ class ConstantsGen < CppGen
       scope("switch (code) {") {
         enum = @amqp.class_(class_name).domain(domain_name).enum
         enum.choices.each { |c|
-          genl "case #{c.value}: holder = new #{c.name.caps}Exception(text); break;" unless c.name == "normal"
+          assign = "holder = new #{c.name.caps}Exception(text); " unless c.name == "normal"
+          genl "case #{c.value}: #{assign}break;" 
         }
-        genl "default: assert(0);"
         genl "    holder = new #{invalid}(QPID_MSG(\"Bad exception code: \" << code << \": \" << text));"
       }
       genl "return holder;"
diff --git a/cpp/src/qpid/client/SessionImpl.cpp b/cpp/src/qpid/client/SessionImpl.cpp
index 5c61248b5a..2d64492bf7 100644
--- a/cpp/src/qpid/client/SessionImpl.cpp
+++ b/cpp/src/qpid/client/SessionImpl.cpp
@@ -54,10 +54,7 @@ typedef sys::ScopedLock<sys::Semaphore>  Acquire;
 SessionImpl::SessionImpl(const std::string& name,
                          shared_ptr<ConnectionImpl> conn,
                          uint16_t ch, uint64_t _maxFrameSize)
-    : error(OK),
-      code(DETACH_CODE_NORMAL),
-      text(EMPTY),
-      state(INACTIVE),
+    : state(INACTIVE),
       detachedLifetime(0),
       maxFrameSize(_maxFrameSize),
       id(conn->getNegotiatedSettings().username, name.empty() ? Uuid(true).str() : name),
@@ -238,17 +235,23 @@ void SessionImpl::markCompleted(const SequenceNumber& id, bool cumulative, bool
     }    
 }
 
+void SessionImpl::setException(const sys::ExceptionHolder& ex) {
+    Lock l(state);
+    setExceptionLH(ex);
+}
+
+void SessionImpl::setExceptionLH(const sys::ExceptionHolder& ex) { // Call with lock held.
+    exceptionHolder = ex;
+    setState(DETACHED);
+}
+
 /**
  * Called by ConnectionImpl to notify active sessions when connection
  * is explictly closed
  */
-void SessionImpl::connectionClosed(uint16_t _code, const std::string& _text) 
-{
-    Lock l(state);
-    error = CONNECTION_CLOSE;
-    code = _code;
-    text = _text;
-    setState(DETACHED);
+void SessionImpl::connectionClosed(uint16_t code, const std::string& text)  {
+    setException(createConnectionException(code, text));
+    // FIXME aconway 2008-10-07: Should closing a connection detach or close its sessions?
     handleClosed();
 }
 
@@ -258,6 +261,7 @@ void SessionImpl::connectionClosed(uint16_t _code, const std::string& _text)
  */
 void SessionImpl::connectionBroke(uint16_t _code, const std::string& _text) 
 {
+    // FIXME aconway 2008-10-07: distinguish disconnect from clean close.  
     connectionClosed(_code, _text);
 }
 
@@ -411,13 +415,12 @@ void SessionImpl::handleIn(AMQFrame& frame) // network thread
                 deliver(frame);
             }
         }
-    } catch (const SessionException& e) {
-        //TODO: proper 0-10 exception handling
-        QPID_LOG(error, "Session exception:" << e.what());
-        Lock l(state);
-        error = EXCEPTION;
-        code = e.code;
-        text = e.what();
+    }
+    catch (const SessionException& e) {
+        setException(createSessionException(e.code, e.getMessage()));
+    }
+    catch (const ChannelException& e) {
+        setException(createChannelException(e.code, e.getMessage()));
     }
 }
 
@@ -478,22 +481,19 @@ void SessionImpl::detach(const std::string& _name)
     QPID_LOG(info, "Session detached by peer: " << id);
 }
 
-void SessionImpl::detached(const std::string& _name, uint8_t _code)
-{
+void SessionImpl::detached(const std::string& _name, uint8_t _code) {
     Lock l(state);
     if (id.getName() != _name) throw InternalErrorException("Incorrect session name");
     setState(DETACHED);
     if (_code) {
         //TODO: make sure this works with execution.exception - don't
         //want to overwrite the code from that
-        QPID_LOG(error, "Session detached by peer: " << id << " " << code);
-        error = SESSION_DETACH;
-        code = _code;
-        text = "Session detached by peer";
+        setExceptionLH(createChannelException(_code, "Session detached by peer"));
+        QPID_LOG(error, exceptionHolder.what());
     }
     if (detachedLifetime == 0) {
         handleClosed();
-    }
+}
 }
 
 void SessionImpl::requestTimeout(uint32_t t)
@@ -606,9 +606,7 @@ void SessionImpl::exception(uint16_t errorCode,
              << " [caused by " << commandId << " " << classCode << ":" << commandCode << "]");
 
     Lock l(state);
-    error = EXCEPTION;
-    code = errorCode;
-    text = description;
+    setExceptionLH(createSessionException(errorCode, description));
     if (detachedLifetime) 
         setTimeout(0);
 }
@@ -631,12 +629,7 @@ inline void SessionImpl::waitFor(State s) //call with lock held
 
 void SessionImpl::check() const  //call with lock held.
 {
-    switch (error) {
-      case OK: break;
-      case CONNECTION_CLOSE: throw ConnectionException(code, text);
-      case SESSION_DETACH: throw ChannelException(code, text);
-      case EXCEPTION: createSessionException(code, text).raise();
-    }
+    exceptionHolder.raise();
 }
 
 void SessionImpl::checkOpen() const  //call with lock held.
@@ -657,7 +650,7 @@ void SessionImpl::handleClosed()
 {
     // FIXME aconway 2008-06-12: needs to be set to the correct exception type.
     // 
-    demux.close(sys::ExceptionHolder(text.empty() ? new ClosedException() : new Exception(text)));
+    demux.close(exceptionHolder.empty() ? new ClosedException() : exceptionHolder);
     results.close();
 }
 
@@ -668,4 +661,5 @@ uint32_t SessionImpl::setTimeout(uint32_t seconds) {
     detachedLifetime = seconds;
     return detachedLifetime;
 }
+
 }}
diff --git a/cpp/src/qpid/client/SessionImpl.h b/cpp/src/qpid/client/SessionImpl.h
index 989294b99e..49a65ed568 100644
--- a/cpp/src/qpid/client/SessionImpl.h
+++ b/cpp/src/qpid/client/SessionImpl.h
@@ -27,6 +27,7 @@
 #include "Results.h"
 
 #include "qpid/SessionId.h"
+#include "qpid/SessionState.h"
 #include "qpid/shared_ptr.h"
 #include "qpid/framing/FrameHandler.h"
 #include "qpid/framing/ChannelHandler.h"
@@ -35,6 +36,7 @@
 #include "qpid/framing/AMQP_ServerProxy.h"
 #include "qpid/sys/Semaphore.h"
 #include "qpid/sys/StateMonitor.h"
+#include "qpid/sys/ExceptionHolder.h"
 
 #include <boost/optional.hpp>
 
@@ -52,6 +54,7 @@ namespace client {
 
 class Future;
 class ConnectionImpl;
+class SessionHandler;
 
 ///@internal
 class SessionImpl : public framing::FrameHandler::InOutHandler,
@@ -91,6 +94,8 @@ public:
     void sendCompletion();
     void sendFlush();
 
+    void setException(const sys::ExceptionHolder&);
+    
     //NOTE: these are called by the network thread when the connection is closed or dies
     void connectionClosed(uint16_t code, const std::string& text);
     void connectionBroke(uint16_t code, const std::string& text);
@@ -102,12 +107,6 @@ public:
     uint32_t getTimeout() const;
 
 private:
-    enum ErrorType {
-        OK,
-        CONNECTION_CLOSE,
-        SESSION_DETACH,
-        EXCEPTION
-    };
     enum State {
         INACTIVE,
         ATTACHING,
@@ -123,6 +122,7 @@ private:
     inline void setState(State s);
     inline void waitFor(State);
 
+    void setExceptionLH(const sys::ExceptionHolder&);      // LH = lock held when called.
     void detach();
     
     void check() const;
@@ -168,9 +168,7 @@ private:
                    const std::string& description,
                    const framing::FieldTable& errorInfo);
 
-    ErrorType error;
-    int code;                   // Error code
-    std::string text;           // Error text
+    sys::ExceptionHolder exceptionHolder;
     mutable StateMonitor state;
     mutable sys::Semaphore sendLock;
     uint32_t detachedLifetime;
@@ -193,6 +191,9 @@ private:
     framing::SequenceNumber nextIn;
     framing::SequenceNumber nextOut;
 
+    SessionState sessionState;
+
+  friend class client::SessionHandler;
 };
 
 }} // namespace qpid::client
diff --git a/cpp/src/qpid/sys/ExceptionHolder.h b/cpp/src/qpid/sys/ExceptionHolder.h
index cfb971411e..fecaa73eea 100644
--- a/cpp/src/qpid/sys/ExceptionHolder.h
+++ b/cpp/src/qpid/sys/ExceptionHolder.h
@@ -52,7 +52,7 @@ class ExceptionHolder : public Raisable {
     template <class Ex> ExceptionHolder& operator=(boost::shared_ptr<Ex> ex) { wrap(ex.release()); return *this; }
         
     void raise() const { if (wrapper.get()) wrapper->raise() ; }
-    std::string what() const { return wrapper->what(); }
+    std::string what() const { return wrapper.get() ? wrapper->what() : std::string(); }
     bool empty() const { return !wrapper.get(); }
     operator bool() const { return !empty(); }
     void reset() { wrapper.reset(); }
diff --git a/cpp/src/tests/exception_test.cpp b/cpp/src/tests/exception_test.cpp
index b42c1e58e0..41061173a7 100644
--- a/cpp/src/tests/exception_test.cpp
+++ b/cpp/src/tests/exception_test.cpp
@@ -83,7 +83,7 @@ QPID_AUTO_TEST_CASE(TestSessionBusy) {
         ScopedSuppressLogging sl; // Suppress messages for expected errors.
         f.connection.newSession(f.session.getId().getName());
         BOOST_FAIL("Expected SessionBusyException for " << f.session.getId().getName());
-    } catch (const Exception&) {} // FIXME aconway 2008-09-22: client is not throwing correct exception.
+    } catch (const SessionBusyException&) {} // FIXME aconway 2008-09-22: client is not throwing correct exception.
 }
 
 QPID_AUTO_TEST_CASE(DisconnectedPop) {
@@ -91,7 +91,7 @@ QPID_AUTO_TEST_CASE(DisconnectedPop) {
     ProxyConnection c(fix.broker->getPort());
     fix.session.queueDeclare(arg::queue="q");
     fix.subs.subscribe(fix.lq, "q");
-    Catcher<Exception> pop(bind(&LocalQueue::pop, boost::ref(fix.lq)));
+    Catcher<ConnectionException> pop(bind(&LocalQueue::pop, boost::ref(fix.lq)));
     fix.connection.proxy.close();
     BOOST_CHECK(pop.join());
 }