PROTON-1618: c++ container: unambiguous listen success/fail indicator Added listener_handler::on_open() to indicate a successful listen.
After a call to container::listen(): - on success, call listener_handler::on_open() before any call to listener_handler::on_accept() - on failure, call listener_handler::on_error() followed by listener_handler::on_close() An application can tell from the first event received (on_open() vs. on_close()) if the listen call succeeded or failed. Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/31d5ba09 Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/31d5ba09 Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/31d5ba09 Branch: refs/heads/master Commit: 31d5ba096aa850e49b0f2475a3c1ef9f171f9a3c Parents: d0b641b Author: Alan Conway <acon...@redhat.com> Authored: Tue Oct 10 16:24:28 2017 -0400 Committer: Alan Conway <acon...@redhat.com> Committed: Wed Oct 11 22:04:42 2017 -0400 ---------------------------------------------------------------------- .../bindings/cpp/include/proton/container.hpp | 12 ++- .../cpp/include/proton/listen_handler.hpp | 3 + proton-c/bindings/cpp/src/container_test.cpp | 89 ++++++++++++-------- .../cpp/src/proactor_container_impl.cpp | 10 ++- 4 files changed, 74 insertions(+), 40 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/31d5ba09/proton-c/bindings/cpp/include/proton/container.hpp ---------------------------------------------------------------------- diff --git a/proton-c/bindings/cpp/include/proton/container.hpp b/proton-c/bindings/cpp/include/proton/container.hpp index d30e45a..fe3857d 100644 --- a/proton-c/bindings/cpp/include/proton/container.hpp +++ b/proton-c/bindings/cpp/include/proton/container.hpp @@ -120,12 +120,16 @@ class PN_CPP_CLASS_EXTERN container { /// Listen for new connections on `listen_url`. /// - /// listen_handler::on_accept is called for each incoming connection to determine + /// If the listener opens successfully, listen_handler::on_open() is called. + /// If it fails to open, listen_handler::on_error() then listen_handler::close() + /// are called. + /// + /// listen_handler::on_accept() is called for each incoming connection to determine /// the @ref connection_options to use, including the @ref messaging_handler. /// - /// **Thread safety** - Calls to `listen_handler` methods - /// are serialized for this listener, but handlers attached to - /// separate listeners can be safely called concurrently. + /// **Thread safety** - Calls to `listen_handler` methods are serialized for + /// this listener, but handlers attached to separate listeners may be called + /// concurrently. PN_CPP_EXTERN listener listen(const std::string& listen_url, listen_handler& handler); http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/31d5ba09/proton-c/bindings/cpp/include/proton/listen_handler.hpp ---------------------------------------------------------------------- diff --git a/proton-c/bindings/cpp/include/proton/listen_handler.hpp b/proton-c/bindings/cpp/include/proton/listen_handler.hpp index 4ce20e9..3e18475 100644 --- a/proton-c/bindings/cpp/include/proton/listen_handler.hpp +++ b/proton-c/bindings/cpp/include/proton/listen_handler.hpp @@ -38,6 +38,9 @@ class listen_handler { public: virtual ~listen_handler() {} + /// Called when the listener is opened successfully. + virtual void on_open(listener&) {} + /// Called for each accepted connection. /// /// Returns connection_options to apply, including a proton::messaging_handler for http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/31d5ba09/proton-c/bindings/cpp/src/container_test.cpp ---------------------------------------------------------------------- diff --git a/proton-c/bindings/cpp/src/container_test.cpp b/proton-c/bindings/cpp/src/container_test.cpp index c2b1609..3b0c86f 100644 --- a/proton-c/bindings/cpp/src/container_test.cpp +++ b/proton-c/bindings/cpp/src/container_test.cpp @@ -35,20 +35,21 @@ namespace { -static std::string int2string(int n) { - std::ostringstream strm; - strm << n; - return strm.str(); +static std::string make_url(const std::string host, int port) { + std::ostringstream url; + url << "amqp://" << host << ":" << port; + return url.str(); } -int listen_on_random_port(proton::container& c, proton::listener& l) { - int port; +int listen_on_random_port(proton::container& c, proton::listener& l, proton::listen_handler* lh=0) { + int port = 0; // I'm going to hell for this: std::srand((unsigned int)time(0)); while (true) { port = 20000 + (std::rand() % 30000); + std::string url = make_url("", port); try { - l = c.listen("0.0.0.0:" + int2string(port)); + l = lh ? c.listen(url, *lh) : c.listen(url); break; } catch (...) { // keep trying @@ -57,6 +58,31 @@ int listen_on_random_port(proton::container& c, proton::listener& l) { return port; } +struct test_listen_handler : public proton::listen_handler { + bool on_open_, on_accept_, on_close_; + std::string on_error_; + test_listen_handler() : on_open_(false), on_accept_(false), on_close_(false) {} + proton::connection_options on_accept(proton::listener&) PN_CPP_OVERRIDE { + on_accept_ = true; + return proton::connection_options(); + } + void on_open(proton::listener&) PN_CPP_OVERRIDE { + on_open_ = true; + ASSERT(!on_accept_); + ASSERT(on_error_.empty()); + ASSERT(!on_close_); + } + void on_close(proton::listener&) PN_CPP_OVERRIDE { + on_close_ = true; + ASSERT(on_open_ || on_error_.size()); + } + + void on_error(proton::listener&, const std::string& e) PN_CPP_OVERRIDE { + on_error_ = e; + ASSERT(!on_close_); + } +}; + class test_handler : public proton::messaging_handler { public: const std::string host; @@ -67,17 +93,21 @@ class test_handler : public proton::messaging_handler { std::string peer_vhost; std::string peer_container_id; proton::listener listener; + test_listen_handler listen_handler; test_handler(const std::string h, const proton::connection_options& c_opts) : host(h), opts(c_opts), closing(false), done(false) {} void on_container_start(proton::container &c) PN_CPP_OVERRIDE { - int port = listen_on_random_port(c, listener); - proton::connection conn = c.connect(host + ":" + int2string(port), opts); + int port = listen_on_random_port(c, listener, &listen_handler); + proton::connection conn = c.connect(make_url(host, port), opts); } void on_connection_open(proton::connection &c) PN_CPP_OVERRIDE { + ASSERT(listen_handler.on_open_); + ASSERT(!listen_handler.on_close_); + ASSERT(listen_handler.on_error_.empty()); if (peer_vhost.empty() && !c.virtual_host().empty()) peer_vhost = c.virtual_host(); if (peer_container_id.empty() && !c.container_id().empty()) @@ -94,26 +124,28 @@ class test_handler : public proton::messaging_handler { int test_container_default_container_id() { proton::connection_options opts; - test_handler th(std::string("127.0.0.1"), opts); + test_handler th("", opts); proton::container(th).run(); ASSERT(!th.peer_container_id.empty()); + ASSERT(th.listen_handler.on_error_.empty()); + ASSERT(th.listen_handler.on_close_); return 0; } int test_container_vhost() { proton::connection_options opts; - opts.virtual_host(std::string("a.b.c")); - test_handler th(std::string("127.0.0.1"), opts); + opts.virtual_host("a.b.c"); + test_handler th("", opts); proton::container(th).run(); - ASSERT_EQUAL(th.peer_vhost, std::string("a.b.c")); + ASSERT_EQUAL(th.peer_vhost, "a.b.c"); return 0; } int test_container_default_vhost() { proton::connection_options opts; - test_handler th(std::string("127.0.0.1"), opts); + test_handler th("127.0.0.1", opts); proton::container(th).run(); - ASSERT_EQUAL(th.peer_vhost, std::string("127.0.0.1")); + ASSERT_EQUAL(th.peer_vhost, "127.0.0.1"); return 0; } @@ -123,25 +155,13 @@ int test_container_no_vhost() { // Sadly whether or not a 'hostname' field was received cannot be // determined from here, so just exercise the code proton::connection_options opts; - opts.virtual_host(std::string("")); - test_handler th(std::string("127.0.0.1"), opts); + opts.virtual_host(""); + test_handler th("127.0.0.1", opts); proton::container(th).run(); - ASSERT_EQUAL(th.peer_vhost, std::string("")); + ASSERT_EQUAL(th.peer_vhost, ""); return 0; } -struct test_listener : public proton::listen_handler { - bool on_accept_, on_close_; - std::string on_error_; - test_listener() : on_accept_(false), on_close_(false) {} - proton::connection_options on_accept(proton::listener&) PN_CPP_OVERRIDE { - on_accept_ = true; - return proton::connection_options(); - } - void on_close(proton::listener&) PN_CPP_OVERRIDE { on_close_ = true; } - void on_error(proton::listener&, const std::string& e) PN_CPP_OVERRIDE { on_error_ = e; } -}; - int test_container_bad_address() { // Listen on a bad address, check for leaks // Regression test for https://issues.apache.org/jira/browse/PROTON-1217 @@ -151,10 +171,11 @@ int test_container_bad_address() { try { c.listen("999.666.999.666:0"); } catch (const proton::error&) {} c.run(); // Dummy listener. - test_listener l; - test_handler h2(std::string("999.999.999.666"), proton::connection_options()); + test_listen_handler l; + test_handler h2("999.999.999.666", proton::connection_options()); try { c.listen("999.666.999.666:0", l); } catch (const proton::error&) {} c.run(); + ASSERT(!l.on_open_); ASSERT(!l.on_accept_); ASSERT(l.on_close_); ASSERT(!l.on_error_.empty()); @@ -168,7 +189,7 @@ class stop_tester : public proton::messaging_handler { void on_container_start(proton::container& c) PN_CPP_OVERRIDE { ASSERT(state==0); int port = listen_on_random_port(c, listener); - c.connect("127.0.0.1:" + int2string(port)); + c.connect(make_url("", port)); c.auto_stop(false); state = 1; } @@ -216,7 +237,7 @@ struct hang_tester : public proton::messaging_handler { hang_tester() : done(false) {} void connect(proton::container* c) { - c->connect("localhost:"+ int2string(port)); + c->connect(make_url("", port)); } void on_container_start(proton::container& c) PN_CPP_OVERRIDE { http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/31d5ba09/proton-c/bindings/cpp/src/proactor_container_impl.cpp ---------------------------------------------------------------------- diff --git a/proton-c/bindings/cpp/src/proactor_container_impl.cpp b/proton-c/bindings/cpp/src/proactor_container_impl.cpp index e83cc66..ec617bd 100644 --- a/proton-c/bindings/cpp/src/proactor_container_impl.cpp +++ b/proton-c/bindings/cpp/src/proactor_container_impl.cpp @@ -498,9 +498,15 @@ bool container::impl::handle(pn_event_t* event) { } return false; } - case PN_LISTENER_OPEN: + case PN_LISTENER_OPEN: { + pn_listener_t* l = pn_event_listener(event); + listener_context &lc(listener_context::get(l)); + if (lc.listen_handler_) { + listener lstnr(l); + lc.listen_handler_->on_open(lstnr); + } return false; - + } case PN_LISTENER_ACCEPT: { pn_listener_t* l = pn_event_listener(event); pn_connection_t* c = pn_connection(); --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org