Repository: qpid-dispatch Updated Branches: refs/heads/master b686f945e -> 452450285
DISPATCH-254 - Fixed a race condition that caused a double free when multiple mnagement queries were in flight at the same time. Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/45245028 Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/45245028 Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/45245028 Branch: refs/heads/master Commit: 4524502856e0c05b3fdbd3705951be89b583e559 Parents: b686f94 Author: Ted Ross <[email protected]> Authored: Mon Apr 4 18:12:01 2016 -0400 Committer: Ted Ross <[email protected]> Committed: Mon Apr 4 18:15:52 2016 -0400 ---------------------------------------------------------------------- src/router_core/agent.c | 19 ++++++++++--------- src/router_core/agent_config_address.c | 2 +- src/router_core/agent_config_auto_link.c | 2 +- src/router_core/agent_config_link_route.c | 2 +- src/router_core/management_agent.c | 14 +++++++------- src/router_core/router_core.c | 1 - src/router_core/router_core_private.h | 1 - 7 files changed, 20 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/45245028/src/router_core/agent.c ---------------------------------------------------------------------- diff --git a/src/router_core/agent.c b/src/router_core/agent.c index 61237ce..0573243 100644 --- a/src/router_core/agent.c +++ b/src/router_core/agent.c @@ -31,6 +31,9 @@ static void qdr_manage_create_CT(qdr_core_t *core, qdr_action_t *action, bool di static void qdr_manage_delete_CT(qdr_core_t *core, qdr_action_t *action, bool discard); static void qdr_manage_update_CT(qdr_core_t *core, qdr_action_t *action, bool discard); +ALLOC_DECLARE(qdr_query_t); +ALLOC_DEFINE(qdr_query_t); + //================================================================================== // Internal Functions //================================================================================== @@ -50,10 +53,10 @@ static void qdr_agent_response_handler(void *context) sys_mutex_unlock(core->query_lock); if (query) { - core->agent_response_handler(query->context, &query->status, query->more); - if (!query->more) { + bool more = query->more; + core->agent_response_handler(query->context, &query->status, more); + if (!more) qdr_query_free(query); - } } } } @@ -226,17 +229,13 @@ void qdr_query_get_next(qdr_query_t *query) void qdr_query_free(qdr_query_t *query) { - if(!query) + if (!query) return; if (query->next_key) qdr_field_free(query->next_key); - if(query->body) - qd_compose_free(query->body); - free_qdr_query_t(query); - } static void qdr_agent_emit_columns(qdr_query_t *query, const char *qdr_columns[], int column_count) @@ -259,7 +258,8 @@ static void qdr_agent_set_columns(qdr_query_t *query, if (!attribute_names || (qd_parse_tag(attribute_names) != QD_AMQP_LIST8 && qd_parse_tag(attribute_names) != QD_AMQP_LIST32) || - qd_parse_sub_count(attribute_names) == 0) { + qd_parse_sub_count(attribute_names) == 0 || + qd_parse_sub_count(attribute_names) >= QDR_AGENT_MAX_COLUMNS) { // // Either the attribute_names field is absent, it's not a list, or it's an empty list. // In this case, we will include all available attributes. @@ -268,6 +268,7 @@ static void qdr_agent_set_columns(qdr_query_t *query, for (i = 0; i < column_count; i++) query->columns[i] = i; query->columns[i] = -1; + assert(i < QDR_AGENT_MAX_COLUMNS); return; } http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/45245028/src/router_core/agent_config_address.c ---------------------------------------------------------------------- diff --git a/src/router_core/agent_config_address.c b/src/router_core/agent_config_address.c index e57b9c4..ace2b58 100644 --- a/src/router_core/agent_config_address.c +++ b/src/router_core/agent_config_address.c @@ -411,5 +411,5 @@ void qdra_config_address_create_CT(qdr_core_t *core, qd_compose_insert_null(query->body); qdr_agent_enqueue_response_CT(core, query); } else - free_qdr_query_t(query); + qdr_query_free(query); } http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/45245028/src/router_core/agent_config_auto_link.c ---------------------------------------------------------------------- diff --git a/src/router_core/agent_config_auto_link.c b/src/router_core/agent_config_auto_link.c index d81c6a4..74ae056 100644 --- a/src/router_core/agent_config_auto_link.c +++ b/src/router_core/agent_config_auto_link.c @@ -420,6 +420,6 @@ void qdra_config_auto_link_create_CT(qdr_core_t *core, } else { if (query->status.status / 100 > 2) qd_log(core->log, QD_LOG_ERROR, "Error configuring linkRoute: %s", query->status.description); - free_qdr_query_t(query); + qdr_query_free(query); } } http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/45245028/src/router_core/agent_config_link_route.c ---------------------------------------------------------------------- diff --git a/src/router_core/agent_config_link_route.c b/src/router_core/agent_config_link_route.c index b3397db..dc3075e 100644 --- a/src/router_core/agent_config_link_route.c +++ b/src/router_core/agent_config_link_route.c @@ -403,6 +403,6 @@ void qdra_config_link_route_create_CT(qdr_core_t *core, } else { if (query->status.status / 100 > 2) qd_log(core->log, QD_LOG_ERROR, "Error configuring linkRoute: %s", query->status.description); - free_qdr_query_t(query); + qdr_query_free(query); } } http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/45245028/src/router_core/management_agent.c ---------------------------------------------------------------------- diff --git a/src/router_core/management_agent.c b/src/router_core/management_agent.c index f39c8c7..dbbcaaf 100644 --- a/src/router_core/management_agent.c +++ b/src/router_core/management_agent.c @@ -157,13 +157,15 @@ static void qd_manage_response_handler(void *context, const qd_amqp_error_t *sta if (ctx->operation_type == QD_ROUTER_OPERATION_QUERY) { if (status->status / 100 == 2) { // There is no error, proceed to conditionally call get_next if (more) { - //If there are no more rows to process or the status returned is something other than - // QD_AMQP_OK, we will close the list, send the message and ctx->current_count++; // Increment how many you have at hand if (ctx->count != ctx->current_count) { qdr_query_get_next(ctx->query); return; - } + } else + // + // This is the one case where the core agent won't free the query itself. + // + qdr_query_free(ctx->query); } } qd_compose_end_list(ctx->field); @@ -176,7 +178,6 @@ static void qd_manage_response_handler(void *context, const qd_amqp_error_t *sta } qd_field_iterator_t *reply_to = 0; - qd_composed_field_t *fld = 0; // Start composing the message. @@ -191,8 +192,6 @@ static void qd_manage_response_handler(void *context, const qd_amqp_error_t *sta qdr_send_to1(ctx->core, ctx->msg, reply_to, true, false); // We have come to the very end. Free the appropriate memory. - // ctx->field has already been freed in the call to qd_compose_end_list(ctx->field) - // ctx->query has also been already freed // Just go over this with Ted to see if I freed everything. qd_field_iterator_free(reply_to); @@ -202,6 +201,7 @@ static void qd_manage_response_handler(void *context, const qd_amqp_error_t *sta qd_message_free(ctx->msg); if (ctx->source) qd_message_free(ctx->source); + qd_compose_free(ctx->field); free_qd_management_context_t(ctx); } @@ -215,7 +215,7 @@ static void qd_core_agent_query_handler(qdr_core_t *core, int *offset) { // - // Add the Body. This body field is freed by qdr_query_free() + // Add the Body. // qd_composed_field_t *field = qd_compose(QD_PERFORMATIVE_BODY_AMQP_VALUE, 0); http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/45245028/src/router_core/router_core.c ---------------------------------------------------------------------- diff --git a/src/router_core/router_core.c b/src/router_core/router_core.c index 020a91c..6562abe 100644 --- a/src/router_core/router_core.c +++ b/src/router_core/router_core.c @@ -20,7 +20,6 @@ #include "router_core_private.h" #include <stdio.h> -ALLOC_DEFINE(qdr_query_t); ALLOC_DEFINE(qdr_address_t); ALLOC_DEFINE(qdr_address_config_t); ALLOC_DEFINE(qdr_node_t); http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/45245028/src/router_core/router_core_private.h ---------------------------------------------------------------------- diff --git a/src/router_core/router_core_private.h b/src/router_core/router_core_private.h index 801eb9a..d65bae2 100644 --- a/src/router_core/router_core_private.h +++ b/src/router_core/router_core_private.h @@ -160,7 +160,6 @@ struct qdr_query_t { qd_amqp_error_t status; }; -ALLOC_DECLARE(qdr_query_t); DEQ_DECLARE(qdr_query_t, qdr_query_list_t); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
