qpid-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From acon...@apache.org
Subject qpid-proton git commit: PROTON-1095: c++: connection_engine error handling, get rid of exceptions.
Date Wed, 27 Jan 2016 23:14:20 GMT
Repository: qpid-proton
Updated Branches:
  refs/heads/master 7b4fa65a2 -> 9c03b28b0


PROTON-1095: c++: connection_engine error handling, get rid of exceptions.

Engine no longer throws any exceptions itself, all connections and transport
errors are reported to the handler which can choose to throw or not.
This is consistent with error handling for the Container.

Added condition::empty()
Removed process_nothrow
Removed disconnect
Return pair<size_t, bool> from io_read rather than throwing on stream close.


Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/9c03b28b
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/9c03b28b
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/9c03b28b

Branch: refs/heads/master
Commit: 9c03b28b0e3865aa892f0fa79650bd6c25759a22
Parents: 7b4fa65
Author: Alan Conway <aconway@redhat.com>
Authored: Wed Jan 27 17:24:47 2016 -0500
Committer: Alan Conway <aconway@redhat.com>
Committed: Wed Jan 27 18:07:09 2016 -0500

----------------------------------------------------------------------
 examples/cpp/engine/broker.cpp                  |  5 +-
 .../bindings/cpp/include/proton/condition.hpp   |  5 +-
 .../bindings/cpp/include/proton/connection.hpp  |  1 -
 .../cpp/include/proton/connection_engine.hpp    | 58 ++++++--------------
 proton-c/bindings/cpp/include/proton/io.hpp     |  2 +-
 proton-c/bindings/cpp/src/condition.cpp         |  6 +-
 proton-c/bindings/cpp/src/connection_engine.cpp | 37 ++++---------
 proton-c/bindings/cpp/src/engine_test.cpp       |  4 +-
 proton-c/bindings/cpp/src/posix/io.cpp          | 13 ++---
 proton-c/bindings/cpp/src/windows/io.cpp        |  9 ++-
 10 files changed, 53 insertions(+), 87 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9c03b28b/examples/cpp/engine/broker.cpp
----------------------------------------------------------------------
diff --git a/examples/cpp/engine/broker.cpp b/examples/cpp/engine/broker.cpp
index 518929c..528bf86 100644
--- a/examples/cpp/engine/broker.cpp
+++ b/examples/cpp/engine/broker.cpp
@@ -133,14 +133,13 @@ class broker {
                     flags |= proton::io::socket_engine::READ;
                 if (FD_ISSET(eng->socket(), &writable_set))
                     flags |= proton::io::socket_engine::WRITE;
-                if (flags) eng->process_nothrow(flags);
+                if (flags) eng->process(flags);
                 // Set reading/writing bits for next time around
                 fd_set_if(eng->can_read(), eng->socket(), &reading_);
                 fd_set_if(eng->can_write(), eng->socket(), &writing_);
 
                 if (eng->closed()) {
-                    std::cout << "closed fd=" << eng->socket() << "
"
-                              << eng->error_str() << std::endl;
+                    std::cout << "closed fd=" << eng->socket() << std::endl;
                     engines_.erase(j);
                     delete eng;
                 }

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9c03b28b/proton-c/bindings/cpp/include/proton/condition.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/condition.hpp b/proton-c/bindings/cpp/include/proton/condition.hpp
index de83c14..0e85c67 100644
--- a/proton-c/bindings/cpp/include/proton/condition.hpp
+++ b/proton-c/bindings/cpp/include/proton/condition.hpp
@@ -40,8 +40,11 @@ class condition {
 
     /// @cond INTERNAL
     /// XXX want to discuss
-    /// Assert no condition set.
+    /// No condition set.
     PN_CPP_EXPORT bool operator!() const;
+
+    /// No condition has been set.
+    PN_CPP_EXPORT bool empty() const;
     /// @endcond
 
     /// Condition name.

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9c03b28b/proton-c/bindings/cpp/include/proton/connection.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/connection.hpp b/proton-c/bindings/cpp/include/proton/connection.hpp
index 410f7f6..75c7338 100644
--- a/proton-c/bindings/cpp/include/proton/connection.hpp
+++ b/proton-c/bindings/cpp/include/proton/connection.hpp
@@ -36,7 +36,6 @@ struct pn_connection_t;
 namespace proton {
 
 class handler;
-class engine;
 
 /// A connection to a remote AMQP peer.
 class connection : public object<pn_connection_t>, public endpoint {

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9c03b28b/proton-c/bindings/cpp/include/proton/connection_engine.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/connection_engine.hpp b/proton-c/bindings/cpp/include/proton/connection_engine.hpp
index 4565f33..367f7aa 100644
--- a/proton-c/bindings/cpp/include/proton/connection_engine.hpp
+++ b/proton-c/bindings/cpp/include/proton/connection_engine.hpp
@@ -110,61 +110,39 @@ class connection_engine {
     /// Read, write and dispatch events.
     ///
     /// io_flags indicates whether to read, write, both or neither.
-    /// dispatches all events generated by reading or writing.
+    /// Dispatches all events generated by reading or writing.
+    /// Use closed() to check if the engine is closed after processing.
     ///
-    /// @throw proton::closed_error if closed() is true before calling process()
-    /// @throw proton::io_error if the engine is closed by an error.
-    /// @return true if process should be called again, i.e. !closed()
-    PN_CPP_EXTERN bool process(int io_flags=READ|WRITE);
-
-    /// Non-throwing version of process.  Use closed() and error_str()
-    /// to check the status of the engine.
-    PN_CPP_EXTERN bool process_nothrow(int io_flags=READ|WRITE);
+    /// @throw exceptions thrown by the engines handler or the IO adapter.
+    PN_CPP_EXTERN void process(int io_flags=READ|WRITE);
 
     /// True if the engine is closed, meaning there are no further
     /// events to process and close_io has been called.  Call
     /// error_str() to get an error description.
     PN_CPP_EXTERN bool closed() const;
 
-    /// If the engine was closed by an error, return a pointer.
-    PN_CPP_EXTERN std::string error_str() const;
-
     /// Get the AMQP connection associated with this connection_engine.
     PN_CPP_EXTERN class connection connection() const;
 
-    /// Get the transport associated with this connection_engine.
-    PN_CPP_EXTERN class transport transport() const;
-
-    /// Disconnect the engine.
+  protected:
+    /// Do a non-blocking read on the IO stream.
     ///
-    /// @internal
+    ///@return pair(size, true) if size bytes were read.
+    /// size==0 means no data could be read without blocking, the stream is still open.
+    /// Returns pair(0, false) if the stream closed.
     ///
-    /// XXX calls io::close?
-    /// 
-    /// Calls io::close and dispatches final events to the
-    /// handler. Neither the handler nor the io will be used after
-    /// this call.
-    PN_CPP_EXTERN void disconnect();
+    ///@throw proton::io_error if there is a read error.
+    virtual std::pair<size_t, bool> io_read(char* buf, size_t max) = 0;
 
-  protected:
-    /** Does a non-blocking read of up to max bytes into buf.
-     * Return the number read, 0 if no data could be read without blocking.
-     *
-     *@throw proton::closed_error if the input reaches EOF.
-     *@throw proton::io_error if there is a read error.
-     */
-    virtual size_t io_read(char* buf, size_t max) = 0;
-
-    /** Does a non-blocking write of up to max bytes from buf.
-     * Return the number written, 0 if no data could be written without blocking.
-     *
-     *@throw proton::io_error if there is a write error.
-     */
+    /// Do a non-blocking write of up to max bytes from buf.
+    ///
+    /// Return the number of byes written , 0 if no data could be written
+    /// without blocking.
+    ///
+    ///throw proton::io_error if there is a write error.
     virtual size_t io_write(const char*, size_t) = 0;
 
-    /**
-     * Close the io, no more _io methods will be called after this is called.
-     */
+    /// Close the io, no more _io methods will be called after this is called.
     virtual void io_close() = 0;
 
     PN_CPP_EXTERN static const connection_options no_opts;

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9c03b28b/proton-c/bindings/cpp/include/proton/io.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/io.hpp b/proton-c/bindings/cpp/include/proton/io.hpp
index cf97f3b..0037c72 100644
--- a/proton-c/bindings/cpp/include/proton/io.hpp
+++ b/proton-c/bindings/cpp/include/proton/io.hpp
@@ -116,7 +116,7 @@ class socket_engine : public connection_engine {
     PN_CPP_EXTERN void run();
 
   protected:
-    PN_CPP_EXTERN size_t io_read(char* buf, size_t max);
+    PN_CPP_EXTERN std::pair<size_t, bool> io_read(char* buf, size_t max);
     PN_CPP_EXTERN size_t io_write(const char*, size_t);
     PN_CPP_EXTERN void io_close();
 

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9c03b28b/proton-c/bindings/cpp/src/condition.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/condition.cpp b/proton-c/bindings/cpp/src/condition.cpp
index 221afcd..b8e3c39 100644
--- a/proton-c/bindings/cpp/src/condition.cpp
+++ b/proton-c/bindings/cpp/src/condition.cpp
@@ -1,5 +1,5 @@
 /*
- * 
+ *
  * 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
@@ -28,6 +28,10 @@ bool condition::operator!() const {
     return !pn_condition_is_set(condition_);
 }
 
+bool condition::empty() const {
+    return !pn_condition_is_set(condition_);
+}
+
 std::string condition::name() const {
     const char* n = pn_condition_get_name(condition_);
     return n ? n : "";

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9c03b28b/proton-c/bindings/cpp/src/connection_engine.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/connection_engine.cpp b/proton-c/bindings/cpp/src/connection_engine.cpp
index 3587a72..89d901e 100644
--- a/proton-c/bindings/cpp/src/connection_engine.cpp
+++ b/proton-c/bindings/cpp/src/connection_engine.cpp
@@ -108,15 +108,8 @@ connection_engine::~connection_engine() {
     pn_collector_free(ctx_->collector);
 }
 
-bool connection_engine::process(int flags) {
-    if (closed()) throw closed_error("engine closed");
-    bool ok = process_nothrow(flags);
-    if (!ok && !error_str().empty()) throw io_error(error_str());
-    return ok;
-}
-
-bool connection_engine::process_nothrow(int flags) {
-    if (closed()) return false;
+void connection_engine::process(int flags) {
+    if (closed()) return;
     if (flags & WRITE) try_write();
     dispatch();
     if (flags & READ) try_read();
@@ -135,7 +128,6 @@ bool connection_engine::process_nothrow(int flags) {
         dispatch();
         try { io_close(); } catch(const io_error&) {} // Tell the IO to close.
     }
-    return !closed();
 }
 
 void connection_engine::dispatch() {
@@ -159,12 +151,14 @@ void connection_engine::try_read() {
     size_t max = can_read();
     if (max == 0) return;
     try {
-        size_t n = io_read(pn_transport_tail(ctx_->transport), max);
-        if (n > max)
-            throw io_error(msg() << "read invalid size: " << n << " >
" << max);
-        pn_transport_process(ctx_->transport, n);
-    } catch (const closed_error&) {
-        pn_transport_close_tail(ctx_->transport);
+        std::pair<size_t, bool> r = io_read(pn_transport_tail(ctx_->transport),
max);
+        if (r.second) {
+            if (r.first > max)
+                throw io_error(msg() << "read invalid size: " << r.first <<
">" << max);
+            pn_transport_process(ctx_->transport, r.first);
+        } else {
+            pn_transport_close_tail(ctx_->transport);
+        }
     } catch (const io_error& e) {
         set_error(ctx_, e.what());
         pn_transport_close_tail(ctx_->transport);
@@ -196,17 +190,6 @@ bool connection_engine::closed() const {
     return pn_transport_closed(ctx_->transport);
 }
 
-std::string connection_engine::error_str() const {
-    pn_condition_t *c = pn_connection_remote_condition(connection_.pn_object());
-    if (!c || !pn_condition_is_set(c)) c = pn_transport_condition(ctx_->transport);
-    if (c && pn_condition_is_set(c)) {
-        std::ostringstream os;
-        os << pn_condition_get_name(c) << ": " << pn_condition_get_description(c);
-        return os.str();
-    }
-    return "";
-}
-
 connection connection_engine::connection() const { return connection_.pn_object(); }
 
 const connection_options connection_engine::no_opts;

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9c03b28b/proton-c/bindings/cpp/src/engine_test.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/engine_test.cpp b/proton-c/bindings/cpp/src/engine_test.cpp
index 2ad36e2..ce71874 100644
--- a/proton-c/bindings/cpp/src/engine_test.cpp
+++ b/proton-c/bindings/cpp/src/engine_test.cpp
@@ -51,12 +51,12 @@ struct mem_engine : public connection_engine {
     mem_engine(mem_pipe s, handler &h, const connection_options &opts)
         : connection_engine(h, opts), socket(s) {}
 
-    size_t io_read(char* buf, size_t size) {
+    std::pair<size_t, bool> io_read(char* buf, size_t size) {
         if (!read_error.empty()) throw io_error(read_error);
         size = std::min(socket.read.size(), size);
         copy(socket.read.begin(), socket.read.begin()+size, buf);
         socket.read.erase(socket.read.begin(), socket.read.begin()+size);
-        return size;
+        return std::make_pair(size, true);
     }
 
     size_t io_write(const char* buf, size_t size) {

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9c03b28b/proton-c/bindings/cpp/src/posix/io.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/posix/io.cpp b/proton-c/bindings/cpp/src/posix/io.cpp
index 6532995..26034a6 100644
--- a/proton-c/bindings/cpp/src/posix/io.cpp
+++ b/proton-c/bindings/cpp/src/posix/io.cpp
@@ -74,16 +74,13 @@ socket_engine::socket_engine(const url& u, handler& h, const connection_options&
     init();
 }
 
-size_t socket_engine::io_read(char *buf, size_t size) {
+std::pair<size_t, bool> socket_engine::io_read(char *buf, size_t size) {
     ssize_t n = ::read(socket_, buf, size);
-    if (n > 0)
-        return n;
-    if (n == 0)
-        throw proton::closed_error();
+    if (n > 0) return std::make_pair(n, true);
+    if (n == 0) return std::make_pair(0, false);
     if (errno == EAGAIN || errno == EWOULDBLOCK)
-        return 0;
-    check(n, "read: ");
-    return n;
+        return std::make_pair(0, true);
+    throw io_error("read: " + error_str());
 }
 
 size_t socket_engine::io_write(const char *buf, size_t size) {

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9c03b28b/proton-c/bindings/cpp/src/windows/io.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/windows/io.cpp b/proton-c/bindings/cpp/src/windows/io.cpp
index ce05e99..32b3d3c 100644
--- a/proton-c/bindings/cpp/src/windows/io.cpp
+++ b/proton-c/bindings/cpp/src/windows/io.cpp
@@ -90,10 +90,13 @@ socket_engine::socket_engine(const url& u, handler& h, const connection_options
     init();
 }
 
-size_t socket_engine::io_read(char *buf, size_t size) {
+std::pair<size_t, bool> socket_engine::io_read(char *buf, size_t size) {
     int n = ::recv(socket_, buf, size, 0);
-    if (n == SOCKET_ERROR && WSAGetLastError() == WSAEWOULDBLOCK) return 0;
-    return check(n, "read: ");
+    if (n > 0) return std::make_pair(n, true);
+    if (n == 0) return std::make_pair(0, false);
+    if (n == SOCKET_ERROR && WSAGetLastError() == WSAEWOULDBLOCK)
+        return std::make_pair(0, true);
+    throw io_error("read: " + error_str());
 }
 
 size_t socket_engine::io_write(const char *buf, size_t size) {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


Mime
View raw message