You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ac...@apache.org on 2018/12/04 17:03:18 UTC
qpid-proton git commit: PROTON-1959: [cpp] Binary compatible reconnect
Repository: qpid-proton
Updated Branches:
refs/heads/master b1ec5464e -> ec97465f8
PROTON-1959: [cpp] Binary compatible reconnect
Previous commit f53c7683d7e90363 added new virtual functions to class
messaging_handler: on_connection_start() and on_connection_reconnecting()
This is a binary incompatible change in C++. Linking a new library that calls
these functions with old user code that doesn't implement them will crash.
Removed the new functions, fixed the implementation and user docs to accomplish
the same result using existing on_connection_open() and on_transport_error().
Fixes #170.
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/ec97465f
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/ec97465f
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/ec97465f
Branch: refs/heads/master
Commit: ec97465f87e844c63cbfcc6007f31ab266dcd494
Parents: b1ec546
Author: Alan Conway <ac...@redhat.com>
Authored: Fri Nov 30 14:07:58 2018 -0500
Committer: Alan Conway <ac...@redhat.com>
Committed: Tue Dec 4 12:02:53 2018 -0500
----------------------------------------------------------------------
cpp/docs/pages.dox | 3 -
cpp/include/proton/messaging_handler.hpp | 118 +++++++++++++++-----------
cpp/src/handler.cpp | 2 -
cpp/src/messaging_adapter.cpp | 3 -
cpp/src/proactor_container_impl.cpp | 16 +++-
cpp/src/reconnect_test.cpp | 43 ++++------
6 files changed, 98 insertions(+), 87 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/ec97465f/cpp/docs/pages.dox
----------------------------------------------------------------------
diff --git a/cpp/docs/pages.dox b/cpp/docs/pages.dox
index c6471f5..818db9b 100644
--- a/cpp/docs/pages.dox
+++ b/cpp/docs/pages.dox
@@ -1,9 +1,6 @@
-/// Order the pages in the documentations side-panel by declaring them here
-
/// @page overview_page Overview
/// @page tutorial_page Tutorial
/// @page connect-config Connection Configuration
/// @page types_page AMQP and C++ types
/// @page mt_page Multi-threading
/// @page io_page IO integration
-///
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/ec97465f/cpp/include/proton/messaging_handler.hpp
----------------------------------------------------------------------
diff --git a/cpp/include/proton/messaging_handler.hpp b/cpp/include/proton/messaging_handler.hpp
index fbefe8e..213dbe7 100644
--- a/cpp/include/proton/messaging_handler.hpp
+++ b/cpp/include/proton/messaging_handler.hpp
@@ -49,43 +49,19 @@ namespace proton {
/// connection::wake() on the other connection before returning. Such
/// a handler is not thread-safe.
///
-/// ### Connection life-cycle and automatic re-connect
+/// ### Overview of connection life-cycle and automatic re-connect
///
-/// on_connection_start() is the first event for any connection.
+/// See the documentation of individual event functions for more details.
///
-/// on_connection_open() means the remote peer has sent an AMQP open.
-/// For a client, this means the connection is fully open. A server
-/// should respond with connection::open() or reject the request with
-/// connection::close()
+/// on_connection_open() - @copybrief on_connection_open()
///
-/// on_connection_reconnecting() may be called if automatic re-connect
-/// is enabled (see reconnect_options). It is called when the
-/// connection is disconnected and a re-connect will be
-/// attempted. Calling connection::close() will cancel the re-connect.
+/// on_transport_error() -@copybrief on_transport_error()
///
-/// on_connection_open() will be called again on a successful
-/// re-connect. Each open @ref session, @ref sender and @ref receiver
-/// will also be automatically re-opened. On success, on_sender_open()
-/// or on_receiver_open() are called, on failure on_sender_error() or
-/// on_receiver_error().
+/// on_connection_error() - @copybrief on_connection_error()
///
-/// on_connection_close() indicates orderly shut-down of the
-/// connection. Servers should respond with connection::close().
-/// on_connection_close() is not called if the connection fails before
-/// the remote end can do an orderly close.
+/// on_connection_close() - @copybrief on_connection_close()
///
-/// on_transport_close() is always the final event for a connection, and
-/// is always called regardless of how the connection closed or failed.
-///
-/// If the connection or transport closes with an error, on_connection_error()
-/// or on_transport_error() is called immediately before on_connection_close() or
-/// on_transport_close(). You can also check for error conditions in the close
-/// function with connection::error() or transport::error()
-///
-/// Note: closing a connection with the special error condition
-/// `amqp:connection-forced`is treated as a disconnect - it triggers
-/// automatic re-connect or on_transport_error()/on_transport_close(),
-/// not on_connection_close().
+/// on_transport_close() - @copybrief on_transport_close()
///
/// @see reconnect_options
///
@@ -116,33 +92,75 @@ PN_CPP_CLASS_EXTERN messaging_handler {
/// The underlying network transport is open
PN_CPP_EXTERN virtual void on_transport_open(transport&);
- /// The underlying network transport has closed.
- /// This is the final event for a connection, there will be
- /// no more events or re-connect attempts.
+ /// The final event for a connection: there will be no more
+ /// reconnect attempts and no more event functions.
+ ///
+ /// Called exactly once regardless of how many times the connection
+ /// was re-connected, whether it failed due to transport or connection
+ /// errors or was closed without error.
+ ///
+ /// This is a good place to do per-connection clean-up.
+ /// transport::connection() is always valid at this point.
+ ///
PN_CPP_EXTERN virtual void on_transport_close(transport&);
- /// The underlying network transport has disconnected unexpectedly.
+ /// Unexpected disconnect, transport::error() provides details; if @ref
+ /// reconnect_options are enabled there may be an automatic re-connect.
+ ///
+ /// If there is a successful automatic reconnect, on_connection_open() will
+ /// be called again.
+ ///
+ /// transport::connection() is always valid at this point.
+ ///
+ /// Calling connection::close() will cancel automatic re-connect and force
+ /// the transport to close immediately, see on_transport_close()
+ ///
PN_CPP_EXTERN virtual void on_transport_error(transport&);
- /// **Unsettled API** - Called before the connection is opened.
- /// Use for initial setup, e.g. to open senders or receivers.
- PN_CPP_EXTERN virtual void on_connection_start(connection&);
-
- /// The remote peer opened the connection.
- /// Called for the initial open, and also after each successful re-connect if
- /// @ref reconnect_options are set.
+ /// The remote peer opened the connection: called once on initial open, and
+ /// again on each successful automatic re-connect (see @ref
+ /// reconnect_options)
+ ///
+ /// connection::reconnected() is false for the initial call, true for
+ /// any subsequent calls due to reconnect.
+ ///
+ /// @note You can use the initial call to open initial @ref session, @ref
+ /// sender and @ref receiver endpoints. All active endpoints are *automatically*
+ /// re-opened after an automatic re-connect so you should take care
+ /// not to open them again, for example:
+ ///
+ /// if (!c.reconnected()) {
+ /// // Do initial per-connection setup here.
+ /// // Open initial senders/receivers if needed.
+ /// } else {
+ /// // Handle a successful reconnect here.
+ /// // NOTE: Don't re-open active senders/receivers
+ /// }
+ ///
+ /// For a server on_connection_open() is called when a client attempts to
+ /// open a connection. The server can call connection::open() to accept or
+ /// connection::close(const error_condition&) to reject.
+ ///
+ /// @see reconnect_options, on_transport_error()
PN_CPP_EXTERN virtual void on_connection_open(connection&);
- /// **Unsettled API** - The connection has been disconnected and
- /// is about to attempt an automatic re-connect.
- /// If on_connection_reconnecting() calls connection::close() then
- /// the reconnect attempt will be canceled.
- PN_CPP_EXTERN virtual void on_connection_reconnecting(connection&);
-
/// The remote peer closed the connection.
+ ///
+ /// If there was a connection error, this is called immediately after
+ /// on_connection_error() and connection::error() is set
+ ///
PN_CPP_EXTERN virtual void on_connection_close(connection&);
- /// The remote peer closed the connection with an error condition.
+ /// The remote peer indicated a fatal error, connection::error() provides details.
+ ///
+ /// A connection error is an application problem, for example an illegal
+ /// action or a lack of resources. There will be no automatic re-connect
+ /// attempts, on_connection_close() will be called immediately after.
+ ///
+ /// @note A connection close with the special error condition
+ /// `amqp:connection-forced` is treated as a transport error, not a
+ /// connection error, see on_transport_error()
+ ///
PN_CPP_EXTERN virtual void on_connection_error(connection&);
/// The remote peer opened the session.
@@ -222,6 +240,6 @@ PN_CPP_CLASS_EXTERN messaging_handler {
PN_CPP_EXTERN virtual void on_error(const error_condition&);
};
-} // proton
+} // namespace proton
#endif // PROTON_MESSAGING_HANDLER_HPP
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/ec97465f/cpp/src/handler.cpp
----------------------------------------------------------------------
diff --git a/cpp/src/handler.cpp b/cpp/src/handler.cpp
index 16fc38e..589b672 100644
--- a/cpp/src/handler.cpp
+++ b/cpp/src/handler.cpp
@@ -56,8 +56,6 @@ void messaging_handler::on_connection_open(connection &c) {
pn_connection_open(unwrap(c));
}
}
-void messaging_handler::on_connection_reconnecting(connection &) {}
-void messaging_handler::on_connection_start(connection &) {}
void messaging_handler::on_connection_wake(connection&) {}
void messaging_handler::on_session_close(session &) {}
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/ec97465f/cpp/src/messaging_adapter.cpp
----------------------------------------------------------------------
diff --git a/cpp/src/messaging_adapter.cpp b/cpp/src/messaging_adapter.cpp
index 91fbc6a..78df0da 100644
--- a/cpp/src/messaging_adapter.cpp
+++ b/cpp/src/messaging_adapter.cpp
@@ -231,9 +231,6 @@ void on_connection_remote_close(messaging_handler& handler, pn_event_t* event) {
void on_connection_bound(messaging_handler& handler, pn_event_t* event) {
connection c(make_wrapper(pn_event_connection(event)));
- if (!c.reconnected()) { // Call on_connection_start() on first connect
- handler.on_connection_start(c);
- }
}
void on_connection_remote_open(messaging_handler& handler, pn_event_t* event) {
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/ec97465f/cpp/src/proactor_container_impl.cpp
----------------------------------------------------------------------
diff --git a/cpp/src/proactor_container_impl.cpp b/cpp/src/proactor_container_impl.cpp
index fe5e087..7e40d4b 100644
--- a/cpp/src/proactor_container_impl.cpp
+++ b/cpp/src/proactor_container_impl.cpp
@@ -670,8 +670,20 @@ container::impl::dispatch_result container::impl::dispatch(pn_event_t* event) {
if (pn_condition_is_set(pn_transport_condition(t)) && can_reconnect(c)) {
messaging_handler *mh = get_handler(event);
if (mh) { // Notify handler of pending reconnect
- connection conn = make_wrapper(c);
- mh->on_connection_reconnecting(conn);
+ transport trans = make_wrapper(t);
+ try {
+ mh->on_transport_error(trans);
+ } catch (const proton::error& e) {
+ // If this is the same error we are re-connecting for,
+ // ignore it. It was probably thrown by the default
+ // messaging_handler::on_error(), and if not the user has
+ // already seen it.
+ //
+ // If this isn't the same error, then something unexpected
+ // has happened, so re-throw.
+ if (std::string(e.what()) != trans.error().what())
+ throw;
+ }
}
// on_connection_reconnecting() may have closed the connection, check again.
if (!(pn_connection_state(c) & PN_LOCAL_CLOSED)) {
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/ec97465f/cpp/src/reconnect_test.cpp
----------------------------------------------------------------------
diff --git a/cpp/src/reconnect_test.cpp b/cpp/src/reconnect_test.cpp
index 5a56cf1..72def2a 100644
--- a/cpp/src/reconnect_test.cpp
+++ b/cpp/src/reconnect_test.cpp
@@ -131,7 +131,7 @@ class server_connection_handler : public proton::messaging_handler {
class tester : public proton::messaging_handler, public waiter {
public:
tester() : waiter(3), container_(*this, "reconnect_client"),
- start_count(0), open_count(0), reconnecting_count(0),
+ start_count(0), open_count(0),
link_open_count(0), transport_error_count(0), transport_close_count(0) {}
void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
@@ -151,23 +151,16 @@ class tester : public proton::messaging_handler, public waiter {
container_.connect(s1->url(), proton::connection_options().reconnect(proton::reconnect_options().failover_urls(urls)));
}
- void on_connection_start(proton::connection& c) PN_CPP_OVERRIDE {
- start_count++;
- c.open_sender("messages");
- ASSERT(!c.reconnected());
- }
-
void on_connection_open(proton::connection& c) PN_CPP_OVERRIDE {
- ASSERT(bool(open_count) == c.reconnected());
+ if (!c.reconnected()) {
+ start_count++;
+ c.open_sender("messages");
+ }
+ ASSERT_EQUAL(bool(open_count), c.reconnected());
open_count++;
}
- void on_connection_reconnecting(proton::connection& c) PN_CPP_OVERRIDE {
- reconnecting_count++;
- }
-
void on_sender_open(proton::sender &s) PN_CPP_OVERRIDE {
- ASSERT(bool(link_open_count) == s.connection().reconnected());
link_open_count++;
}
@@ -180,6 +173,7 @@ class tester : public proton::messaging_handler, public waiter {
}
void on_transport_error(proton::transport& t) PN_CPP_OVERRIDE {
+ ASSERT_EQUAL(bool(transport_error_count), t.connection().reconnected());
transport_error_count++;
}
@@ -191,11 +185,10 @@ class tester : public proton::messaging_handler, public waiter {
container_.run();
ASSERT_EQUAL(1, start_count);
ASSERT_EQUAL(3, open_count);
- ASSERT(2 < reconnecting_count);
+ // Could be > 3, unpredicatble number reconnects while listener comes up.
+ ASSERT(2 < transport_error_count);
// Last reconnect fails before opening links
ASSERT(link_open_count > 1);
- // All transport errors should have been hidden
- ASSERT_EQUAL(0, transport_error_count);
// One final transport close, not an error
ASSERT_EQUAL(1, transport_close_count);
}
@@ -205,7 +198,7 @@ class tester : public proton::messaging_handler, public waiter {
proton::internal::pn_unique_ptr<server_connection_handler> s2;
proton::internal::pn_unique_ptr<server_connection_handler> s3;
proton::container container_;
- int start_count, open_count, reconnecting_count, link_open_count, transport_error_count, transport_close_count;
+ int start_count, open_count, link_open_count, transport_error_count, transport_close_count;
};
int test_failover_simple() {
@@ -284,11 +277,11 @@ class authfail_reconnect_tester : public proton::messaging_handler, public waite
bool errored_;
};
-// Verify we can stop reconnecting by calling close() in on_connection_reconnecting()
+// Verify we can stop reconnecting by calling close() in on_transport_error()
class test_reconnecting_close : public proton::messaging_handler, public waiter {
public:
test_reconnecting_close() : waiter(1), container_(*this, "test_reconnecting_close"),
- reconnecting_called(false) {}
+ transport_error_called(false) {}
void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
s1.reset(new server_connection_handler(c, 0, *this));
@@ -298,19 +291,15 @@ class test_reconnecting_close : public proton::messaging_handler, public waiter
container_.connect(s1->url(), proton::connection_options().reconnect(proton::reconnect_options()));
}
- void on_connection_reconnecting(proton::connection& c) PN_CPP_OVERRIDE {
- reconnecting_called = true;
- c.close(); // Abort reconnection
+ void on_transport_error(proton::transport& t) PN_CPP_OVERRIDE {
+ transport_error_called = true;
+ t.connection().close(); // Abort reconnection
}
void on_connection_close(proton::connection& c) PN_CPP_OVERRIDE {
ASSERT(0); // Not expecting any clean close
}
- void on_transport_error(proton::transport& t) PN_CPP_OVERRIDE {
- // Expected, don't throw
- }
-
void run() {
container_.run();
}
@@ -318,7 +307,7 @@ class test_reconnecting_close : public proton::messaging_handler, public waiter
private:
proton::container container_;
std::string err_;
- bool reconnecting_called;
+ bool transport_error_called;
proton::internal::pn_unique_ptr<server_connection_handler> s1;
};
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org