Repository: qpid-proton Updated Branches: refs/heads/master 8d0c5afd0 -> 9a7b2cf03
PROTON-1217: Sporadic memory leak in C++ container_test Fixed missing call to listen_handler::on_closed in the event of an exception. Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/9a7b2cf0 Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/9a7b2cf0 Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/9a7b2cf0 Branch: refs/heads/master Commit: 9a7b2cf03d195e6647bc5ee32189c794326c8951 Parents: 8d0c5af Author: Alan Conway <[email protected]> Authored: Tue May 31 14:22:10 2016 -0400 Committer: Alan Conway <[email protected]> Committed: Tue May 31 14:22:10 2016 -0400 ---------------------------------------------------------------------- .../cpp/include/proton/default_container.hpp | 3 ++ proton-c/bindings/cpp/src/container.cpp | 5 +++- proton-c/bindings/cpp/src/container_impl.cpp | 10 ++++--- proton-c/bindings/cpp/src/container_test.cpp | 31 ++++++++++++++++++++ 4 files changed, 44 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9a7b2cf0/proton-c/bindings/cpp/include/proton/default_container.hpp ---------------------------------------------------------------------- diff --git a/proton-c/bindings/cpp/include/proton/default_container.hpp b/proton-c/bindings/cpp/include/proton/default_container.hpp index da47163..f50869c 100644 --- a/proton-c/bindings/cpp/include/proton/default_container.hpp +++ b/proton-c/bindings/cpp/include/proton/default_container.hpp @@ -54,6 +54,7 @@ class PN_CPP_CLASS_EXTERN default_container : public container { PN_CPP_EXTERN returned<connection> connect(const std::string& url, const connection_options &) PN_CPP_OVERRIDE; PN_CPP_EXTERN listener listen(const std::string& url, listen_handler& l) PN_CPP_OVERRIDE; + using container::listen; /// @cond INTERNAL /// XXX Make private @@ -69,11 +70,13 @@ class PN_CPP_CLASS_EXTERN default_container : public container { const std::string &url, const proton::sender_options &o, const connection_options &c) PN_CPP_OVERRIDE; + using container::open_sender; PN_CPP_EXTERN returned<receiver> open_receiver( const std::string&url, const proton::receiver_options &o, const connection_options &c) PN_CPP_OVERRIDE; + using container::open_receiver; PN_CPP_EXTERN std::string id() const PN_CPP_OVERRIDE; http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9a7b2cf0/proton-c/bindings/cpp/src/container.cpp ---------------------------------------------------------------------- diff --git a/proton-c/bindings/cpp/src/container.cpp b/proton-c/bindings/cpp/src/container.cpp index c053889..d8a6061 100644 --- a/proton-c/bindings/cpp/src/container.cpp +++ b/proton-c/bindings/cpp/src/container.cpp @@ -80,7 +80,10 @@ namespace{ } listener container::listen(const std::string& url, const connection_options& opts) { - return listen(url, *new listen_opts(opts)); + // Note: listen_opts::on_close() calls delete(this) so this is not a leak. + // The container will always call on_closed() even if there are errors or exceptions. + listen_opts* lh = new listen_opts(opts); + return listen(url, *lh); } listener container::listen(const std::string &url) { http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9a7b2cf0/proton-c/bindings/cpp/src/container_impl.cpp ---------------------------------------------------------------------- diff --git a/proton-c/bindings/cpp/src/container_impl.cpp b/proton-c/bindings/cpp/src/container_impl.cpp index b7bc9df..ca12727 100644 --- a/proton-c/bindings/cpp/src/container_impl.cpp +++ b/proton-c/bindings/cpp/src/container_impl.cpp @@ -201,10 +201,12 @@ listener container_impl::listen(const std::string& url, listen_handler& lh) { proton::url u(url); pn_acceptor_t *acptr = pn_reactor_acceptor( reactor_.pn_object(), u.host().c_str(), u.port().c_str(), chandler.get()); - if (!acptr) - throw error(MSG("accept fail: " << - pn_error_text(pn_io_error(reactor_.pn_io()))) - << "(" << url << ")"); + if (!acptr) { + std::string err(pn_error_text(pn_io_error(reactor_.pn_io()))); + lh.on_error(err); + lh.on_close(); + throw error(err); + } // Do not use pn_acceptor_set_ssl_domain(). Manage the incoming connections ourselves for // more flexibility (i.e. ability to change the server cert for a long running listener). listener_context& lc(listener_context::get(acptr)); http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9a7b2cf0/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 b8ab40a..940f833 100644 --- a/proton-c/bindings/cpp/src/container_test.cpp +++ b/proton-c/bindings/cpp/src/container_test.cpp @@ -24,6 +24,7 @@ #include "proton/default_container.hpp" #include "proton/messaging_handler.hpp" #include "proton/listener.hpp" +#include "proton/listen_handler.hpp" #include <cstdlib> #include <ctime> @@ -116,6 +117,35 @@ int test_container_no_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() PN_CPP_OVERRIDE { + on_accept_ = true; + return proton::connection_options(); + } + void on_close() PN_CPP_OVERRIDE { on_close_ = true; } + void on_error(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 + + proton::default_container c; + // Default fixed-option listener. Valgrind for leaks. + try { c.listen("999.666.999.666:0"); } catch (const proton::error&) {} + // Dummy listener. + test_listener l; + test_handler h2(std::string("999.999.999.666"), proton::connection_options()); + try { c.listen("999.666.999.666:0", l); } catch (const proton::error&) {} + ASSERT(!l.on_accept_); + ASSERT(l.on_close_); + ASSERT(!l.on_error_.empty()); + return 0; +} + } int main(int, char**) { @@ -123,6 +153,7 @@ int main(int, char**) { RUN_TEST(failed, test_container_vhost()); RUN_TEST(failed, test_container_default_vhost()); RUN_TEST(failed, test_container_no_vhost()); + RUN_TEST(failed, test_container_bad_address()); return failed; } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
