modified reference semantics so that child objects don't need to be explicitly freed
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/99df0a33 Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/99df0a33 Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/99df0a33 Branch: refs/heads/master Commit: 99df0a33dabdcefb3ec327e9bd3051f53590c660 Parents: 70c4112 Author: Rafael Schloming <[email protected]> Authored: Tue Jan 13 15:02:27 2015 -0500 Committer: Rafael Schloming <[email protected]> Committed: Tue Jan 13 16:23:59 2015 -0500 ---------------------------------------------------------------------- proton-c/bindings/python/proton/__init__.py | 18 +++---- proton-c/include/proton/link.h | 15 +----- proton-c/include/proton/session.h | 19 ++----- proton-c/src/engine/engine-internal.h | 2 - proton-c/src/engine/engine.c | 64 ++++++++---------------- proton-c/src/tests/reactor.c | 3 -- proton-c/src/tests/refcount.c | 5 ++ proton-c/src/transport/transport.c | 6 --- proton-j/src/main/resources/cengine.py | 4 +- 9 files changed, 41 insertions(+), 95 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/99df0a33/proton-c/bindings/python/proton/__init__.py ---------------------------------------------------------------------- diff --git a/proton-c/bindings/python/proton/__init__.py b/proton-c/bindings/python/proton/__init__.py index 537276b..cb2dc28 100644 --- a/proton-c/bindings/python/proton/__init__.py +++ b/proton-c/bindings/python/proton/__init__.py @@ -2356,11 +2356,8 @@ class Connection(Wrapper, Endpoint): def state(self): return pn_connection_state(self._impl) - def _pn_session(self): - return pn_session(self._impl) - def session(self): - return Session(self._pn_session) + return Session(pn_session(self._impl)) def session_head(self, mask): return Session.wrap(pn_session_head(self._impl, mask)) @@ -2435,13 +2432,13 @@ class Session(Wrapper, Endpoint): return Connection.wrap(pn_session_connection(self._impl)) def sender(self, name): - return Sender(lambda: pn_sender(self._impl, name)) + return Sender(pn_sender(self._impl, name)) def receiver(self, name): - return Receiver(lambda: pn_receiver(self._impl, name)) + return Receiver(pn_receiver(self._impl, name)) def free(self): - pn_session_release(self._impl) + pn_session_free(self._impl) class LinkException(ProtonException): pass @@ -2514,7 +2511,7 @@ class Link(Wrapper, Endpoint): return self.session.connection def delivery(self, tag): - return Delivery(lambda: pn_delivery(self._impl, tag)) + return Delivery(pn_delivery(self._impl, tag)) @property def current(self): @@ -2581,7 +2578,7 @@ class Link(Wrapper, Endpoint): return pn_link_detach(self._impl) def free(self): - pn_link_release(self._impl) + pn_link_free(self._impl) class Terminus(object): @@ -2865,9 +2862,6 @@ class Delivery(Wrapper): return pn_delivery_settled(self._impl) def settle(self): - # XXX: we have to incref here because settle is overloaded to - # decref also, remove this when C semantics are fixed - pn_incref(self._impl) pn_delivery_settle(self._impl) @property http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/99df0a33/proton-c/include/proton/link.h ---------------------------------------------------------------------- diff --git a/proton-c/include/proton/link.h b/proton-c/include/proton/link.h index c320b75..531c399 100644 --- a/proton-c/include/proton/link.h +++ b/proton-c/include/proton/link.h @@ -77,25 +77,14 @@ PN_EXTERN pn_link_t *pn_receiver(pn_session_t *session, const char *name); * Free a link object. * * When a link object is freed, all ::pn_delivery_t objects associated - * with the session are also freed. + * with the session are also freed. Freeing a link will settle any + * unsettled deliveries on the link. * * @param[in] link a link object to free (or NULL) */ PN_EXTERN void pn_link_free(pn_link_t *link); /** - * Releasing a link object. - * - * When a link object is freed, it will no longer be retained by its - * session once all internal references to the link are no longer - * needed. Releasing a link will settle any unsettled deliveries on - * the link. - * - * @param[in] link the link to be released - */ -PN_EXTERN void pn_link_release(pn_link_t *link); - -/** * @deprecated * Get the application context that is associated with a link object. * http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/99df0a33/proton-c/include/proton/session.h ---------------------------------------------------------------------- diff --git a/proton-c/include/proton/session.h b/proton-c/include/proton/session.h index 4e3bbb5..b283fce 100644 --- a/proton-c/include/proton/session.h +++ b/proton-c/include/proton/session.h @@ -53,25 +53,14 @@ PN_EXTERN pn_session_t *pn_session(pn_connection_t *connection); /** * Free a session object. * - * When a session object is freed, all ::pn_link_t, and - * ::pn_delivery_t objects associated with the session are also - * freed. - * - * @param[in] session a session object to free (or NULL) - */ -PN_EXTERN void pn_session_free(pn_session_t *session); - -/** - * Release a session object. - * - * When a session is released it will no longer be retained by the + * When a session is freed it will no longer be retained by the * connection once any internal references to the session are no - * longer needed. Releasing a session will release all links on that + * longer needed. Freeing a session will free all links on that * session and settle any deliveries on those links. * - * @param[in] session a session object to release + * @param[in] session a session object to free (or NULL) */ -PN_EXTERN void pn_session_release(pn_session_t *session); +PN_EXTERN void pn_session_free(pn_session_t *session); /** * @deprecated http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/99df0a33/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 02fd8bc..5655e79 100644 --- a/proton-c/src/engine/engine-internal.h +++ b/proton-c/src/engine/engine-internal.h @@ -52,8 +52,6 @@ struct pn_endpoint_t { int refcount; // when this hits zero we generate a final event bool modified; bool freed; - bool constructed; // track whether the endpoint was explicitly - // constructed or not bool referenced; }; http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/99df0a33/proton-c/src/engine/engine.c ---------------------------------------------------------------------- diff --git a/proton-c/src/engine/engine.c b/proton-c/src/engine/engine.c index 6573e94..4d56100 100644 --- a/proton-c/src/engine/engine.c +++ b/proton-c/src/engine/engine.c @@ -107,10 +107,7 @@ void pn_connection_close(pn_connection_t *connection) void pn_endpoint_tini(pn_endpoint_t *endpoint); -static void pn_session_free2(pn_session_t *session, bool decref); -static void pn_link_free2(pn_link_t *link, bool decref); - -static void pn_connection_free2(pn_connection_t *connection, bool decref) +void pn_connection_release(pn_connection_t *connection) { assert(!connection->endpoint.freed); // free those endpoints that haven't been freed by the application @@ -120,11 +117,11 @@ static void pn_connection_free2(pn_connection_t *connection, bool decref) switch (ep->type) { case SESSION: // note: this will free all child links: - pn_session_free2((pn_session_t *)ep, decref); + pn_session_free((pn_session_t *)ep); break; case SENDER: case RECEIVER: - pn_link_free2((pn_link_t *)ep, decref); + pn_link_free((pn_link_t *)ep); break; default: assert(false); @@ -138,17 +135,11 @@ static void pn_connection_free2(pn_connection_t *connection, bool decref) pn_connection_unbound(connection); } pn_ep_decref(&connection->endpoint); - if (decref) { - pn_decref(connection); - } } void pn_connection_free(pn_connection_t *connection) { - pn_connection_free2(connection, true); -} - -void pn_connection_release(pn_connection_t *connection) { - pn_connection_free2(connection, false); + pn_connection_release(connection); + pn_decref(connection); } void pn_connection_bound(pn_connection_t *connection) @@ -252,12 +243,12 @@ void pn_session_close(pn_session_t *session) pn_endpoint_close(&session->endpoint); } -static void pn_session_free2(pn_session_t *session, bool decref) +void pn_session_free(pn_session_t *session) { assert(!session->endpoint.freed); while(pn_list_size(session->links)) { pn_link_t *link = (pn_link_t *)pn_list_get(session->links, 0); - pn_link_free2(link, decref); + pn_link_free(link); } pn_remove_session(session->connection, session); pn_list_add(session->connection->freed, session); @@ -265,17 +256,11 @@ static void pn_session_free2(pn_session_t *session, bool decref) LL_REMOVE(pn_ep_get_connection(endpoint), endpoint, endpoint); session->endpoint.freed = true; pn_ep_decref(&session->endpoint); - if (decref && session->endpoint.constructed) { - pn_decref(session); - } -} - -void pn_session_free(pn_session_t *session) { - pn_session_free2(session, true); -} -void pn_session_release(pn_session_t *session) { - pn_session_free2(session, false); + // the finalize logic depends on endpoint.freed, so we incref/decref + // to give it a chance to rerun + pn_incref(session); + pn_decref(session); } pn_record_t *pn_session_attachments(pn_session_t *session) @@ -340,7 +325,7 @@ void pn_terminus_free(pn_terminus_t *terminus) pn_free(terminus->filter); } -static void pn_link_free2(pn_link_t *link, bool decref) +void pn_link_free(pn_link_t *link) { assert(!link->endpoint.freed); pn_endpoint_t *endpoint = (pn_endpoint_t *) link; @@ -350,25 +335,16 @@ static void pn_link_free2(pn_link_t *link, bool decref) pn_delivery_t *delivery = link->unsettled_head; while (delivery) { pn_delivery_t *next = delivery->unsettled_next; - if (!(decref && delivery->constructed)) { - pn_incref(delivery); - } pn_delivery_settle(delivery); delivery = next; } link->endpoint.freed = true; pn_ep_decref(&link->endpoint); - if (decref && link->endpoint.constructed) { - pn_decref(link); - } -} -void pn_link_free(pn_link_t *link) { - pn_link_free2(link, true); -} - -void pn_link_release(pn_link_t *link) { - pn_link_free2(link, false); + // the finalize logic depends on endpoint.freed (modified above), so + // we incref/decref to give it a chance to rerun + pn_incref(link); + pn_decref(link); } void *pn_link_get_context(pn_link_t *link) @@ -403,7 +379,6 @@ void pn_endpoint_init(pn_endpoint_t *endpoint, int type, pn_connection_t *conn) endpoint->transport_prev = NULL; endpoint->modified = false; endpoint->freed = false; - endpoint->constructed = true; endpoint->refcount = 1; //fprintf(stderr, "initting 0x%lx\n", (uintptr_t) endpoint); @@ -963,6 +938,7 @@ pn_session_t *pn_session(pn_connection_t *conn) if (conn->transport) { pn_session_bound(ssn); } + pn_decref(ssn); return ssn; } @@ -1126,6 +1102,7 @@ pn_link_t *pn_link_new(int type, pn_session_t *session, const char *name) if (session->connection->transport) { pn_link_bound(link); } + pn_decref(link); return link; } @@ -1475,7 +1452,6 @@ pn_delivery_t *pn_delivery(pn_link_t *link, pn_delivery_tag_t tag) pn_buffer_clear(delivery->bytes); delivery->done = false; pn_record_clear(delivery->context); - delivery->constructed = true; // begin delivery state delivery->state.init = false; @@ -1489,6 +1465,9 @@ pn_delivery_t *pn_delivery(pn_link_t *link, pn_delivery_tag_t tag) pn_work_update(link->session->connection, delivery); + // XXX: could just remove incref above + pn_decref(delivery); + return delivery; } @@ -1775,6 +1754,7 @@ void pn_delivery_settle(pn_delivery_t *delivery) delivery->local.settled = true; pn_add_tpwork(delivery); pn_work_update(delivery->link->session->connection, delivery); + pn_incref(delivery); pn_decref(delivery); } } http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/99df0a33/proton-c/src/tests/reactor.c ---------------------------------------------------------------------- diff --git a/proton-c/src/tests/reactor.c b/proton-c/src/tests/reactor.c index bc4eeb3..4eff533 100644 --- a/proton-c/src/tests/reactor.c +++ b/proton-c/src/tests/reactor.c @@ -333,9 +333,6 @@ void source_dispatch(pn_handler_t *handler, pn_event_t *event) { pn_connection_open(conn); pn_session_open(ssn); pn_link_open(snd); - // XXX: these keep the connection alive even when we release it - pn_decref(ssn); - pn_decref(snd); } break; case PN_LINK_FLOW: http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/99df0a33/proton-c/src/tests/refcount.c ---------------------------------------------------------------------- diff --git a/proton-c/src/tests/refcount.c b/proton-c/src/tests/refcount.c index 87ce488..3c563ad 100644 --- a/proton-c/src/tests/refcount.c +++ b/proton-c/src/tests/refcount.c @@ -39,7 +39,9 @@ #define SETUP_CSL \ pn_connection_t *conn = pn_connection(); \ pn_session_t *ssn = pn_session(conn); \ + pn_incref(ssn); \ pn_link_t *lnk = pn_sender(ssn, "sender"); \ + pn_incref(lnk); \ \ assert(pn_refcount(conn) == 2); \ assert(pn_refcount(ssn) == 2); \ @@ -172,8 +174,11 @@ static void swap(int array[], int i, int j) { static void setup(void **objects) { pn_connection_t *conn = pn_connection(); pn_session_t *ssn = pn_session(conn); + pn_incref(ssn); pn_link_t *lnk = pn_sender(ssn, "sender"); + pn_incref(lnk); pn_delivery_t *dlv = pn_delivery(lnk, pn_dtag("dtag", 4)); + pn_incref(dlv); assert(pn_refcount(conn) == 2); assert(pn_refcount(ssn) == 2); http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/99df0a33/proton-c/src/transport/transport.c ---------------------------------------------------------------------- diff --git a/proton-c/src/transport/transport.c b/proton-c/src/transport/transport.c index 1a1e969..84ae566 100644 --- a/proton-c/src/transport/transport.c +++ b/proton-c/src/transport/transport.c @@ -971,8 +971,6 @@ int pn_do_begin(pn_transport_t *transport, uint8_t frame_type, uint16_t channel, ssn = (pn_session_t *) pn_hash_get(transport->local_channels, remote_channel); } else { ssn = pn_session(transport->connection); - ssn->endpoint.constructed = false; - pn_decref(ssn); } ssn->state.incoming_transfer_count = next; pni_map_remote_channel(ssn, channel); @@ -1086,8 +1084,6 @@ int pn_do_attach(pn_transport_t *transport, uint8_t frame_type, uint16_t channel } else { link = (pn_link_t *) pn_receiver(ssn, strname); } - link->endpoint.constructed = false; - pn_decref(link); } if (strheap) { @@ -1209,8 +1205,6 @@ int pn_do_transfer(pn_transport_t *transport, uint8_t frame_type, uint16_t chann } delivery = pn_delivery(link, pn_dtag(tag.start, tag.size)); - delivery->constructed = false; - pn_decref(delivery); pn_delivery_state_t *state = pn_delivery_map_push(incoming, delivery); if (id_present && id != state->id) { return pn_do_error(transport, "amqp:session:invalid-field", http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/99df0a33/proton-j/src/main/resources/cengine.py ---------------------------------------------------------------------- diff --git a/proton-j/src/main/resources/cengine.py b/proton-j/src/main/resources/cengine.py index 3730bec..8742873 100644 --- a/proton-j/src/main/resources/cengine.py +++ b/proton-j/src/main/resources/cengine.py @@ -331,7 +331,7 @@ def pn_sender(ssn, name): def pn_receiver(ssn, name): return wrap(ssn.impl.receiver(name), pn_link_wrapper) -def pn_session_release(ssn): +def pn_session_free(ssn): ssn.impl.free() TERMINUS_TYPES_J2P = { @@ -663,7 +663,7 @@ def pn_link_advance(link): def pn_link_current(link): return wrap(link.impl.current(), pn_delivery_wrapper) -def pn_link_release(link): +def pn_link_free(link): link.impl.free() def pn_work_head(conn): --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
