This is an automated email from the ASF dual-hosted git repository.
kgiusti pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git
The following commit(s) were added to refs/heads/master by this push:
new 11756ad DISPATCH-1214: remove shutdown leak of references reported by
Valgrind
11756ad is described below
commit 11756ad277c571f75dcea26f2819211097a7d478
Author: Kenneth Giusti <kgiu...@apache.org>
AuthorDate: Tue Jan 15 16:04:53 2019 -0500
DISPATCH-1214: remove shutdown leak of references reported by Valgrind
This closes #439
---
src/router_core/connections.c | 12 +++++++++-
src/router_core/route_control.c | 38 +++----------------------------
src/router_core/route_control.h | 1 +
src/router_core/router_core.c | 50 ++++++++++++++++++++++++++++++++++++++---
4 files changed, 62 insertions(+), 39 deletions(-)
diff --git a/src/router_core/connections.c b/src/router_core/connections.c
index 155a469..6184f52 100644
--- a/src/router_core/connections.c
+++ b/src/router_core/connections.c
@@ -855,13 +855,22 @@ static void qdr_link_cleanup_CT(qdr_core_t *core,
qdr_connection_t *conn, qdr_li
qdr_link_cleanup_deliveries_CT(core, conn, link);
//
- // Remove the reference to this link in the connection's reference lists
+ // Remove all references to this link in the connection's and owning
+ // address reference lists
//
sys_mutex_lock(conn->work_lock);
qdr_del_link_ref(&conn->links, link, QDR_LINK_LIST_CLASS_CONNECTION);
qdr_del_link_ref(conn->links_with_work + link->priority, link,
QDR_LINK_LIST_CLASS_WORK);
sys_mutex_unlock(conn->work_lock);
+ if (link->ref[QDR_LINK_LIST_CLASS_ADDRESS]) {
+ assert(link->owning_addr);
+ qdr_del_link_ref((link->link_direction == QD_OUTGOING)
+ ? &link->owning_addr->rlinks
+ : &link->owning_addr->inlinks,
+ link, QDR_LINK_LIST_CLASS_ADDRESS);
+ }
+
//
// Free the link's name and terminus_addr
//
@@ -1256,6 +1265,7 @@ static void qdr_detach_link_control_CT(qdr_core_t *core,
qdr_connection_t *conn,
{
if (conn->role == QDR_ROLE_INTER_ROUTER) {
qdr_del_link_ref(&core->hello_addr->rlinks, link,
QDR_LINK_LIST_CLASS_ADDRESS);
+ link->owning_addr = 0;
core->control_links_by_mask_bit[conn->mask_bit] = 0;
qdr_post_link_lost_CT(core, conn->mask_bit);
}
diff --git a/src/router_core/route_control.c b/src/router_core/route_control.c
index 30cfb44..4810b98 100644
--- a/src/router_core/route_control.c
+++ b/src/router_core/route_control.c
@@ -91,7 +91,7 @@ static qdr_conn_identifier_t
*qdr_route_declare_id_CT(qdr_core_t *core,
}
-static void qdr_route_check_id_for_deletion_CT(qdr_core_t *core,
qdr_conn_identifier_t *cid)
+void qdr_route_check_id_for_deletion_CT(qdr_core_t *core,
qdr_conn_identifier_t *cid)
{
//
// If this connection identifier has no open connection and no referencing
routes,
@@ -428,8 +428,7 @@ void qdr_route_auto_link_closed_CT(qdr_core_t *core,
qdr_link_t *link)
void qdr_route_del_link_route_CT(qdr_core_t *core, qdr_link_route_t *lr)
{
//
- // Disassociate from the connection identifier. Check to see if the
identifier
- // should be removed.
+ // Deactivate the link route on all connections
//
qdr_conn_identifier_t *cid = lr->conn_id;
if (cid) {
@@ -438,19 +437,9 @@ void qdr_route_del_link_route_CT(qdr_core_t *core,
qdr_link_route_t *lr)
qdr_link_route_deactivate_CT(core, lr, cref->conn);
cref = DEQ_NEXT(cref);
}
- DEQ_REMOVE_N(REF, cid->link_route_refs, lr);
- qdr_route_check_id_for_deletion_CT(core, cid);
}
//
- // Disassociate the link route from its address. Check to see if the
address
- // (and its associated pattern) should be removed.
- //
- qdr_address_t *addr = lr->addr;
- if (addr && --addr->ref_count == 0)
- qdr_check_addr_CT(core, addr);
-
- //
// Remove the link route from the core list.
//
DEQ_REMOVE(core->link_routes, lr);
@@ -541,26 +530,13 @@ void qdr_route_del_auto_link_CT(qdr_core_t *core,
qdr_auto_link_t *al)
qdr_auto_link_deactivate_CT(core, al, cref->conn);
cref = DEQ_NEXT(cref);
}
- DEQ_REMOVE_N(REF, cid->auto_link_refs, al);
- qdr_route_check_id_for_deletion_CT(core, cid);
}
//
- // Disassociate the auto link from its address. Check to see if the
address
- // should be removed.
- //
- qdr_address_t *addr = al->addr;
- if (addr && --addr->ref_count == 0)
- qdr_check_addr_CT(core, addr);
-
- //
// Remove the auto link from the core list.
//
DEQ_REMOVE(core->auto_links, al);
- free(al->name);
- free(al->external_addr);
- qdr_core_timer_free_CT(core, al->retry_timer);
- free_qdr_auto_link_t(al);
+ qdr_core_delete_auto_link(core, al);
}
@@ -755,14 +731,6 @@ void qdr_route_del_conn_route_CT(qdr_core_t *core,
qdr_link_route_deactivate_CT(core, lr, conn);
//
- // Disassociate the link route from its address. Check to see if the
address
- // (and its associated pattern) should be removed.
- //
- qdr_address_t *addr = lr->addr;
- if (addr && --addr->ref_count == 0)
- qdr_check_addr_CT(core, addr);
-
- //
// Remove the link route from the parent's link route list
//
DEQ_REMOVE(conn->conn_link_routes, lr);
diff --git a/src/router_core/route_control.h b/src/router_core/route_control.h
index 7d893a3..8a1039b 100644
--- a/src/router_core/route_control.h
+++ b/src/router_core/route_control.h
@@ -54,6 +54,7 @@ void qdr_route_connection_closed_CT(qdr_core_t *core,
qdr_connection_t *conn);
void qdr_link_route_map_pattern_CT(qdr_core_t *core, qd_iterator_t *address,
qdr_address_t *addr);
void qdr_link_route_unmap_pattern_CT(qdr_core_t *core, qd_iterator_t *address);
+void qdr_route_check_id_for_deletion_CT(qdr_core_t *core,
qdr_conn_identifier_t *cid);
/**
* Actions to be performed when an auto link detaches.
diff --git a/src/router_core/router_core.c b/src/router_core/router_core.c
index 7a9a8ee..6d458fe 100644
--- a/src/router_core/router_core.c
+++ b/src/router_core/router_core.c
@@ -116,7 +116,6 @@ void qdr_core_free(qdr_core_t *core)
sys_mutex_free(core->id_lock);
qd_timer_free(core->work_timer);
-
//we can't call qdr_core_unsubscribe on the subscriptions because the
action processing thread has
//already been shut down. But, all the action would have done at this
point is free the subscriptions
//so we just do that directly.
@@ -155,8 +154,6 @@ void qdr_core_free(qdr_core_t *core)
qd_parse_tree_free(core->addr_parse_tree);
qd_parse_tree_free(core->link_route_tree[QD_INCOMING]);
qd_parse_tree_free(core->link_route_tree[QD_OUTGOING]);
- qd_hash_free(core->conn_id_hash);
- //TODO what about the actual connection identifier objects?
qdr_node_t *rnode = 0;
while ( (rnode = DEQ_HEAD(core->routers)) ) {
@@ -166,6 +163,8 @@ void qdr_core_free(qdr_core_t *core)
qdr_link_t *link = DEQ_HEAD(core->open_links);
while (link) {
DEQ_REMOVE_HEAD(core->open_links);
+ qdr_del_link_ref(&link->conn->links, link,
QDR_LINK_LIST_CLASS_CONNECTION);
+ qdr_del_link_ref(&link->conn->links_with_work[link->priority], link,
QDR_LINK_LIST_CLASS_WORK);
free(link->name);
free(link->disambiguated_name);
free(link->terminus_addr);
@@ -180,10 +179,17 @@ void qdr_core_free(qdr_core_t *core)
qdr_connection_t *conn = DEQ_HEAD(core->open_connections);
while (conn) {
DEQ_REMOVE_HEAD(core->open_connections);
+ if (conn->conn_id) {
+ qdr_del_connection_ref(&conn->conn_id->connection_refs, conn);
+ qdr_route_check_id_for_deletion_CT(core, conn->conn_id);
+ }
qdr_connection_free(conn);
conn = DEQ_HEAD(core->open_connections);
}
+ // at this point all the conn identifiers have been freed
+ qd_hash_free(core->conn_id_hash);
+
if (core->query_lock) sys_mutex_free(core->query_lock);
if (core->routers_by_mask_bit) free(core->routers_by_mask_bit);
if (core->control_links_by_mask_bit) free(core->control_links_by_mask_bit);
@@ -411,6 +417,17 @@ bool qdr_is_addr_treatment_multicast(qdr_address_t *addr)
void qdr_core_delete_link_route(qdr_core_t *core, qdr_link_route_t *lr)
{
+ if (lr->conn_id) {
+ DEQ_REMOVE_N(REF, lr->conn_id->link_route_refs, lr);
+ qdr_route_check_id_for_deletion_CT(core, lr->conn_id);
+ }
+
+ if (lr->addr) {
+ if (--lr->addr->ref_count == 0) {
+ qdr_check_addr_CT(core, lr->addr);
+ }
+ }
+
free(lr->add_prefix);
free(lr->del_prefix);
free(lr->name);
@@ -420,6 +437,15 @@ void qdr_core_delete_link_route(qdr_core_t *core,
qdr_link_route_t *lr)
void qdr_core_delete_auto_link(qdr_core_t *core, qdr_auto_link_t *al)
{
+ if (al->conn_id) {
+ DEQ_REMOVE_N(REF, al->conn_id->auto_link_refs, al);
+ qdr_route_check_id_for_deletion_CT(core, al->conn_id);
+ }
+
+ qdr_address_t *addr = al->addr;
+ if (addr && --addr->ref_count == 0)
+ qdr_check_addr_CT(core, addr);
+
free(al->name);
free(al->external_addr);
qdr_core_timer_free_CT(core, al->retry_timer);
@@ -453,6 +479,17 @@ void qdr_core_remove_address(qdr_core_t *core,
qdr_address_t *addr)
}
// Free resources associated with this address
+
+ DEQ_APPEND(addr->rlinks, addr->inlinks);
+ qdr_link_ref_t *lref = DEQ_HEAD(addr->rlinks);
+ while (lref) {
+ qdr_link_t *link = lref->link;
+ assert(link->owning_addr == addr);
+ link->owning_addr = 0;
+ qdr_del_link_ref(&addr->rlinks, link, QDR_LINK_LIST_CLASS_ADDRESS);
+ lref = DEQ_HEAD(addr->rlinks);
+ }
+
qd_bitmask_free(addr->rnodes);
if (addr->treatment == QD_TREATMENT_ANYCAST_CLOSEST) {
qd_bitmask_free(addr->closest_remotes);
@@ -460,6 +497,13 @@ void qdr_core_remove_address(qdr_core_t *core,
qdr_address_t *addr)
else if (addr->treatment == QD_TREATMENT_ANYCAST_BALANCED) {
free(addr->outstanding_deliveries);
}
+
+ qdr_connection_ref_t *cr = DEQ_HEAD(addr->conns);
+ while (cr) {
+ qdr_del_connection_ref(&addr->conns, cr->conn);
+ cr = DEQ_HEAD(addr->conns);
+ }
+
free(addr->add_prefix);
free(addr->del_prefix);
free_qdr_address_t(addr);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org
For additional commands, e-mail: commits-h...@qpid.apache.org
