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 2015/11/26 23:25:26 UTC

[2/2] qpid-proton git commit: NO-JIRA: c++: Memory leaks after object refactor.

NO-JIRA: c++: Memory leaks after object refactor.

- Consistent class/struct declaration for connection_context (clang complains)
- Removed bogus 'operator delete' on reactor, left over by mistake.
- Make connection_context::default_session a pn_session_t*
  - using a proton::session was creating a refcount cycle.
- decref in reactor::create - application is taking owership.
- blocking_connection_impl: add missing reactor_.stop() in destructor.


Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/f1b484ac
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/f1b484ac
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/f1b484ac

Branch: refs/heads/master
Commit: f1b484ac6c9afd030f23c20252412e64366f92ba
Parents: 1e04209
Author: Alan Conway <ac...@redhat.com>
Authored: Thu Nov 26 17:04:32 2015 -0500
Committer: Alan Conway <ac...@redhat.com>
Committed: Thu Nov 26 17:24:57 2015 -0500

----------------------------------------------------------------------
 proton-c/bindings/cpp/include/proton/connection.hpp    | 2 +-
 proton-c/bindings/cpp/include/proton/reactor.hpp       | 2 --
 proton-c/bindings/cpp/src/blocking_connection_impl.cpp | 4 +++-
 proton-c/bindings/cpp/src/connection.cpp               | 7 +++++--
 proton-c/bindings/cpp/src/connector.cpp                | 4 +++-
 proton-c/bindings/cpp/src/contexts.hpp                 | 2 +-
 proton-c/bindings/cpp/src/reactor.cpp                  | 9 ++++-----
 7 files changed, 17 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/f1b484ac/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 bb438ed..cc8c5ba 100644
--- a/proton-c/bindings/cpp/include/proton/connection.hpp
+++ b/proton-c/bindings/cpp/include/proton/connection.hpp
@@ -34,7 +34,7 @@ struct pn_connection_t;
 
 namespace proton {
 
-class connection_context;
+struct connection_context;
 class handler;
 class engine;
 

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/f1b484ac/proton-c/bindings/cpp/include/proton/reactor.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/reactor.hpp b/proton-c/bindings/cpp/include/proton/reactor.hpp
index 929a24b..5f628ce 100644
--- a/proton-c/bindings/cpp/include/proton/reactor.hpp
+++ b/proton-c/bindings/cpp/include/proton/reactor.hpp
@@ -87,8 +87,6 @@ class reactor : public object<pn_reactor_t> {
     PN_CPP_EXTERN void yield();
 
     void container_context(container&);
-
-    void operator delete(void*);
 };
 
 }

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/f1b484ac/proton-c/bindings/cpp/src/blocking_connection_impl.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/blocking_connection_impl.cpp b/proton-c/bindings/cpp/src/blocking_connection_impl.cpp
index 7a5d882..e0b7c93 100644
--- a/proton-c/bindings/cpp/src/blocking_connection_impl.cpp
+++ b/proton-c/bindings/cpp/src/blocking_connection_impl.cpp
@@ -55,7 +55,9 @@ blocking_connection_impl::blocking_connection_impl(const url& url, duration time
     wait(connection_opening(connection_));
 }
 
-blocking_connection_impl::~blocking_connection_impl() {}
+blocking_connection_impl::~blocking_connection_impl() {
+    container_->reactor().stop();
+}
 
 void blocking_connection_impl::close() {
     connection_.close();

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/f1b484ac/proton-c/bindings/cpp/src/connection.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/connection.cpp b/proton-c/bindings/cpp/src/connection.cpp
index 5226a12..733face 100644
--- a/proton-c/bindings/cpp/src/connection.cpp
+++ b/proton-c/bindings/cpp/src/connection.cpp
@@ -90,8 +90,11 @@ session connection::open_session() { return pn_session(pn_object()); }
 session connection::default_session() {
     struct connection_context& ctx = connection_context::get(pn_object());
     if (!ctx.default_session) {
-        ctx.default_session = open_session();
-        ctx.default_session.open();
+        // Note we can't use a proton::session here because we don't want to own
+        // a session reference. The connection owns the session, owning it here as well
+        // would create a circular ownership.
+        ctx.default_session = pn_session(pn_object());
+        pn_session_open(ctx.default_session);
     }
     return ctx.default_session;
 }

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/f1b484ac/proton-c/bindings/cpp/src/connector.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/connector.cpp b/proton-c/bindings/cpp/src/connector.cpp
index 2547ff5..90a16e6 100644
--- a/proton-c/bindings/cpp/src/connector.cpp
+++ b/proton-c/bindings/cpp/src/connector.cpp
@@ -60,8 +60,10 @@ void connector::reconnect_timer(const class reconnect_timer &rt) {
 void connector::connect() {
     connection_.container_id(connection_.container().id());
     connection_.host(address_.host_port());
-    transport t(pn_transport());
+    pn_transport_t *pnt = pn_transport();
+    transport t(pnt);
     t.bind(connection_);
+    pn_decref((void *)pnt);
     // Apply options to the new transport.
     options_.apply(connection_);
     transport_configured_ = true;

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/f1b484ac/proton-c/bindings/cpp/src/contexts.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/contexts.hpp b/proton-c/bindings/cpp/src/contexts.hpp
index 34df0ce..a0e6cf6 100644
--- a/proton-c/bindings/cpp/src/contexts.hpp
+++ b/proton-c/bindings/cpp/src/contexts.hpp
@@ -45,7 +45,7 @@ struct connection_context : public counted {
     ~connection_context();
 
     pn_unique_ptr<class handler> handler;
-    session default_session;   // Owned by connection
+    pn_session_t *default_session;   // Owned by connection
     class container_impl* container_impl;
     message event_message;  // re-used by messaging_adapter for performance
 };

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/f1b484ac/proton-c/bindings/cpp/src/reactor.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/reactor.cpp b/proton-c/bindings/cpp/src/reactor.cpp
index 31d3275..9a52d99 100644
--- a/proton-c/bindings/cpp/src/reactor.cpp
+++ b/proton-c/bindings/cpp/src/reactor.cpp
@@ -30,7 +30,10 @@
 namespace proton {
 
 reactor reactor::create() {
-    return reactor(pn_reactor());
+    pn_reactor_t *p = pn_reactor();
+    reactor r(p);
+    pn_decref(p);               // FIXME aconway 2015-11-26: take ownership
+    return r;
 }
 
 void reactor::run() { pn_reactor_run(pn_object()); }
@@ -93,8 +96,4 @@ void reactor::container_context(container& c) {
     proton::container_context(pn_object(), c);
 }
 
-void reactor::operator delete(void* p) {
-    pn_reactor_free(reinterpret_cast<pn_reactor_t*>(p));
-}
-
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org