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 2015/01/13 22:26:15 UTC
[3/3] qpid-proton git commit: modified reference semantics so that
child objects don't need to be explicitly freed
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 <rh...@alum.mit.edu>
Authored: Tue Jan 13 15:02:27 2015 -0500
Committer: Rafael Schloming <rh...@alum.mit.edu>
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: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org