You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by rh...@apache.org on 2014/11/27 12:31:20 UTC
[3/3] qpid-proton git commit: fixed refcounting semantics to allow
navigation from parent to child without reference cycles
fixed refcounting semantics to allow navigation from parent to child without reference cycles
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/1781b4e9
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/1781b4e9
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/1781b4e9
Branch: refs/heads/master
Commit: 1781b4e99f7ee376162877bc02fe85eb39a7e1d5
Parents: fb3ecd4
Author: Rafael Schloming <rh...@alum.mit.edu>
Authored: Tue Nov 25 08:52:24 2014 -0500
Committer: Rafael Schloming <rh...@alum.mit.edu>
Committed: Thu Nov 27 06:14:50 2014 -0500
----------------------------------------------------------------------
proton-c/include/proton/link.h | 1 +
proton-c/src/engine/engine-internal.h | 1 +
proton-c/src/engine/engine.c | 98 +++++++++++++++-
proton-c/src/tests/CMakeLists.txt | 2 +-
proton-c/src/tests/refcount.c | 176 +++++++++++++++++++++++++++++
5 files changed, 271 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/1781b4e9/proton-c/include/proton/link.h
----------------------------------------------------------------------
diff --git a/proton-c/include/proton/link.h b/proton-c/include/proton/link.h
index 863fe16..245cce5 100644
--- a/proton-c/include/proton/link.h
+++ b/proton-c/include/proton/link.h
@@ -24,6 +24,7 @@
#include <proton/import_export.h>
#include <proton/type_compat.h>
+#include <proton/terminus.h>
#include <stddef.h>
#include <sys/types.h>
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/1781b4e9/proton-c/src/engine/engine-internal.h
----------------------------------------------------------------------
diff --git a/proton-c/src/engine/engine-internal.h b/proton-c/src/engine/engine-internal.h
index fbeb79b..16ce6a4 100644
--- a/proton-c/src/engine/engine-internal.h
+++ b/proton-c/src/engine/engine-internal.h
@@ -51,6 +51,7 @@ struct pn_endpoint_t {
pn_endpoint_t *transport_prev;
bool modified;
bool freed;
+ bool referenced;
bool posted_final;
};
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/1781b4e9/proton-c/src/engine/engine.c
----------------------------------------------------------------------
diff --git a/proton-c/src/engine/engine.c b/proton-c/src/engine/engine.c
index 79da48c..96c601f 100644
--- a/proton-c/src/engine/engine.c
+++ b/proton-c/src/engine/engine.c
@@ -336,6 +336,7 @@ pn_record_t *pn_link_attachments(pn_link_t *link)
void pn_endpoint_init(pn_endpoint_t *endpoint, int type, pn_connection_t *conn)
{
endpoint->type = (pn_endpoint_type_t) type;
+ endpoint->referenced = true;
endpoint->state = PN_LOCAL_UNINIT | PN_REMOTE_UNINIT;
endpoint->error = pn_error();
pn_condition_init(&endpoint->condition);
@@ -371,6 +372,17 @@ static bool pni_post_final(pn_endpoint_t *endpoint, pn_event_type_t type)
return false;
}
+static void pni_free_children(pn_list_t *children)
+{
+ while (pn_list_size(children) > 0) {
+ pn_endpoint_t *endpoint = (pn_endpoint_t *) pn_list_get(children, 0);
+ assert(!endpoint->referenced);
+ pn_free(endpoint);
+ }
+
+ pn_free(children);
+}
+
static void pn_connection_finalize(void *object)
{
pn_connection_t *conn = (pn_connection_t *) object;
@@ -380,9 +392,10 @@ static void pn_connection_finalize(void *object)
return;
}
+ pni_free_children(conn->sessions);
pn_free(conn->context);
pn_decref(conn->collector);
- pn_free(conn->sessions);
+
pn_free(conn->container);
pn_free(conn->hostname);
pn_free(conn->offered_capabilities);
@@ -713,6 +726,34 @@ pn_link_t *pn_link_next(pn_link_t *link, pn_state_t state)
return NULL;
}
+static void pn_session_incref(void *object)
+{
+ pn_session_t *session = (pn_session_t *) object;
+ if (!session->endpoint.referenced) {
+ session->endpoint.referenced = true;
+ pn_incref(session->connection);
+ } else {
+ pn_object_incref(object);
+ }
+}
+
+static bool pni_preserve_child(pn_endpoint_t *endpoint, pn_endpoint_t *parent)
+{
+ if (!endpoint->freed && endpoint->referenced) {
+ pn_object_incref(endpoint);
+ endpoint->referenced = false;
+ pn_decref(parent);
+ return true;
+ } else {
+ return false;
+ }
+}
+
+static bool pni_connection_live(pn_connection_t *conn)
+{
+ return pn_refcount(conn) > 1;
+}
+
static void pn_session_finalize(void *object)
{
pn_session_t *session = (pn_session_t *) object;
@@ -729,16 +770,28 @@ static void pn_session_finalize(void *object)
return;
}
+ if (pni_connection_live(session->connection) &&
+ pni_preserve_child(endpoint, &session->connection->endpoint)) {
+ return;
+ }
+
pn_free(session->context);
- pn_free(session->links);
+ pni_free_children(session->links);
pn_endpoint_tini(endpoint);
pn_delivery_map_free(&session->state.incoming);
pn_delivery_map_free(&session->state.outgoing);
pn_free(session->state.local_handles);
pn_free(session->state.remote_handles);
- pn_decref(session->connection);
+ pn_remove_session(session->connection, session);
+ if (endpoint->referenced) {
+ pn_decref(session->connection);
+ }
}
+#define pn_session_new pn_object_new
+#define pn_session_refcount pn_object_refcount
+#define pn_session_decref pn_object_decref
+#define pn_session_reify pn_object_reify
#define pn_session_initialize NULL
#define pn_session_hashcode NULL
#define pn_session_compare NULL
@@ -747,7 +800,9 @@ static void pn_session_finalize(void *object)
pn_session_t *pn_session(pn_connection_t *conn)
{
assert(conn);
- static const pn_class_t clazz = PN_CLASS(pn_session);
+#define pn_session_free pn_object_free
+ static const pn_class_t clazz = PN_METACLASS(pn_session);
+#undef pn_session_free
pn_session_t *ssn = (pn_session_t *) pn_class_new(&clazz, sizeof(pn_session_t));
if (!ssn) return NULL;
@@ -836,6 +891,22 @@ void pn_terminus_init(pn_terminus_t *terminus, pn_terminus_type_t type)
terminus->filter = pn_data(0);
}
+static void pn_link_incref(void *object)
+{
+ pn_link_t *link = (pn_link_t *) object;
+ if (!link->endpoint.referenced) {
+ link->endpoint.referenced = true;
+ pn_incref(link->session);
+ } else {
+ pn_object_incref(object);
+ }
+}
+
+static bool pni_session_live(pn_session_t *ssn)
+{
+ return pni_connection_live(ssn->connection) || pn_refcount(ssn) > 1;
+}
+
static void pn_link_finalize(void *object)
{
pn_link_t *link = (pn_link_t *) object;
@@ -849,6 +920,11 @@ static void pn_link_finalize(void *object)
return;
}
+ if (pni_session_live(link->session) &&
+ pni_preserve_child(endpoint, &link->session->endpoint)) {
+ return;
+ }
+
pn_free(link->context);
pn_terminus_free(&link->source);
pn_terminus_free(&link->target);
@@ -856,9 +932,15 @@ static void pn_link_finalize(void *object)
pn_terminus_free(&link->remote_target);
pn_free(link->name);
pn_endpoint_tini(endpoint);
- pn_decref(link->session);
+ pn_remove_link(link->session, link);
+ if (endpoint->referenced) {
+ pn_decref(link->session);
+ }
}
+#define pn_link_refcount pn_object_refcount
+#define pn_link_decref pn_object_decref
+#define pn_link_reify pn_object_reify
#define pn_link_initialize NULL
#define pn_link_hashcode NULL
#define pn_link_compare NULL
@@ -866,7 +948,11 @@ static void pn_link_finalize(void *object)
pn_link_t *pn_link_new(int type, pn_session_t *session, const char *name)
{
- static const pn_class_t clazz = PN_CLASS(pn_link);
+#define pn_link_new pn_object_new
+#define pn_link_free pn_object_free
+ static const pn_class_t clazz = PN_METACLASS(pn_link);
+#undef pn_link_new
+#undef pn_link_free
pn_link_t *link = (pn_link_t *) pn_class_new(&clazz, sizeof(pn_link_t));
pn_endpoint_init(&link->endpoint, type, session->connection);
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/1781b4e9/proton-c/src/tests/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/proton-c/src/tests/CMakeLists.txt b/proton-c/src/tests/CMakeLists.txt
index 98bc205..fb7df1b 100644
--- a/proton-c/src/tests/CMakeLists.txt
+++ b/proton-c/src/tests/CMakeLists.txt
@@ -44,4 +44,4 @@ pn_add_c_test (c-object-tests object.c)
pn_add_c_test (c-message-tests message.c)
pn_add_c_test (c-engine-tests engine.c)
pn_add_c_test (c-parse-url-tests parse-url.c)
-
+pn_add_c_test (c-refcount-tests refcount.c)
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/1781b4e9/proton-c/src/tests/refcount.c
----------------------------------------------------------------------
diff --git a/proton-c/src/tests/refcount.c b/proton-c/src/tests/refcount.c
new file mode 100644
index 0000000..2c1cd0c
--- /dev/null
+++ b/proton-c/src/tests/refcount.c
@@ -0,0 +1,176 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.h>
+#include <proton/session.h>
+#include <proton/link.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#define assert(E) ((E) ? 0 : (abort(), 0))
+
+/**
+ * The decref order tests validate that whenever the last pointer to a
+ * child object, e.g. a session or a link, is about to go away, the
+ * parent object takes ownership of that reference if the child object
+ * has not been freed, this avoids reference cycles but allows
+ * navigation from parents to children.
+ **/
+
+#define SETUP_CSL \
+ pn_connection_t *conn = pn_connection(); \
+ pn_session_t *ssn = pn_session(conn); \
+ pn_link_t *lnk = pn_sender(ssn, "sender"); \
+ \
+ assert(pn_refcount(conn) == 2); \
+ assert(pn_refcount(ssn) == 2); \
+ assert(pn_refcount(lnk) == 1);
+
+static void test_decref_order_csl(void) {
+ SETUP_CSL;
+
+ pn_decref(conn);
+ assert(pn_refcount(conn) == 1); // session keeps alive
+ pn_decref(ssn);
+ assert(pn_refcount(ssn) == 1); // link keeps alive
+ pn_decref(lnk);
+ // all gone now (requires valgrind to validate)
+}
+
+static void test_decref_order_cls(void) {
+ SETUP_CSL;
+
+ pn_decref(conn);
+ assert(pn_refcount(conn) == 1); // session keeps alive
+ pn_decref(lnk);
+ assert(pn_refcount(lnk) == 1); // session takes over ownership
+ pn_decref(ssn);
+ // all gone now (requires valgrind to validate)
+}
+
+static void test_decref_order_lcs(void) {
+ SETUP_CSL;
+
+ pn_decref(lnk);
+ assert(pn_refcount(lnk) == 1); // session takes over ownership
+ pn_decref(conn);
+ assert(pn_refcount(conn) == 1); // session keeps alive
+ pn_decref(ssn);
+ // all gone now (requires valgrind to validate)
+}
+
+static void test_decref_order_scl(void) {
+ SETUP_CSL;
+
+ pn_decref(ssn);
+ assert(pn_refcount(ssn) == 1); // link keeps alive
+ pn_decref(conn);
+ assert(pn_refcount(conn) == 1); // session keeps alive
+ pn_decref(lnk);
+ // all gone now (requires valgrind to validate)
+}
+
+static void test_decref_order_slc(void) {
+ SETUP_CSL;
+
+ pn_decref(ssn);
+ assert(pn_refcount(ssn) == 1); // link keeps alive
+ pn_decref(lnk);
+ assert(pn_refcount(ssn) == 1); // connection takes over ownership
+ assert(pn_refcount(lnk) == 1); // session takes over ownership
+ pn_decref(conn);
+ // all gone now (requires valgrind to validate)
+}
+
+static void test_decref_order_lsc(void) {
+ SETUP_CSL;
+
+ pn_decref(lnk);
+ assert(pn_refcount(lnk) == 1); // session takes over ownership
+ assert(pn_refcount(ssn) == 1);
+ pn_decref(ssn);
+ assert(pn_refcount(lnk) == 1);
+ assert(pn_refcount(ssn) == 1); // connection takes over ownership
+ pn_decref(conn);
+ // all gone now (requires valgrind to validate)
+}
+
+/**
+ * The incref order tests verify that once ownership of the last
+ * pointer to a child is taken over by a parent, it is reassigned when
+ * the child is increfed.
+ **/
+
+#define SETUP_INCREF_ORDER \
+ SETUP_CSL; \
+ pn_decref(lnk); \
+ pn_decref(ssn); \
+ assert(pn_refcount(lnk) == 1); \
+ assert(pn_refcount(ssn) == 1); \
+ assert(pn_refcount(conn) == 1);
+
+static void test_incref_order_sl(void) {
+ SETUP_INCREF_ORDER;
+
+ pn_incref(ssn);
+ assert(pn_refcount(conn) == 2);
+ assert(pn_refcount(ssn) == 1);
+ assert(pn_refcount(lnk) == 1);
+ pn_incref(lnk);
+ assert(pn_refcount(conn) == 2);
+ assert(pn_refcount(ssn) == 2);
+ assert(pn_refcount(lnk) == 1);
+
+ pn_decref(conn);
+ pn_decref(ssn);
+ pn_decref(lnk);
+}
+
+static void test_incref_order_ls(void) {
+ SETUP_INCREF_ORDER;
+
+ pn_incref(lnk);
+ assert(pn_refcount(conn) == 2);
+ assert(pn_refcount(ssn) == 1);
+ assert(pn_refcount(lnk) == 1);
+ pn_incref(ssn);
+ assert(pn_refcount(conn) == 2);
+ assert(pn_refcount(ssn) == 2);
+ assert(pn_refcount(lnk) == 1);
+
+ pn_decref(conn);
+ pn_decref(ssn);
+ pn_decref(lnk);
+}
+
+int main(int argc, char **argv)
+{
+ test_decref_order_csl();
+ test_decref_order_cls();
+ test_decref_order_lcs();
+ test_decref_order_scl();
+ test_decref_order_slc();
+ test_decref_order_lsc();
+
+ test_incref_order_sl();
+ test_incref_order_ls();
+ return 0;
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org