Return-Path: X-Original-To: apmail-qpid-commits-archive@www.apache.org Delivered-To: apmail-qpid-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 2636618921 for ; Wed, 27 Jan 2016 23:14:21 +0000 (UTC) Received: (qmail 10514 invoked by uid 500); 27 Jan 2016 23:14:21 -0000 Delivered-To: apmail-qpid-commits-archive@qpid.apache.org Received: (qmail 10484 invoked by uid 500); 27 Jan 2016 23:14:21 -0000 Mailing-List: contact commits-help@qpid.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@qpid.apache.org Delivered-To: mailing list commits@qpid.apache.org Received: (qmail 10470 invoked by uid 99); 27 Jan 2016 23:14:21 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 27 Jan 2016 23:14:21 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id E82A0E00B2; Wed, 27 Jan 2016 23:14:20 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: aconway@apache.org To: commits@qpid.apache.org Message-Id: <07647b7fcffc4e4d977b535176892ad0@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: qpid-proton git commit: PROTON-1095: c++: connection_engine error handling, get rid of exceptions. Date: Wed, 27 Jan 2016 23:14:20 +0000 (UTC) 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 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 Authored: Wed Jan 27 17:24:47 2016 -0500 Committer: Alan Conway 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, 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 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 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 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 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 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 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