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 2016/05/31 18:25:48 UTC
qpid-proton git commit: PROTON-1217: Sporadic memory leak in C++
container_test
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 <ac...@redhat.com>
Authored: Tue May 31 14:22:10 2016 -0400
Committer: Alan Conway <ac...@redhat.com>
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: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org