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 <[email protected]> Authored: Tue Nov 25 08:52:24 2014 -0500 Committer: Rafael Schloming <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
