DISPATCH-254 - Fixed a race condition that caused a double free when multiple mnagement
queries were in flight at the same time.
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_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 @@
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_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 @@
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 @@
for (i = 0; i < column_count; i++)
query->columns[i] = i;
query->columns[i] = -1;
+ assert(i < QDR_AGENT_MAX_COLUMNS);
return;
}
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 @@
qd_compose_insert_null(query->body);
qdr_agent_enqueue_response_CT(core, query);
} else
- free_qdr_query_t(query);
+ qdr_query_free(query);
}
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 @@
} 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);
}
}
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 @@
} 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);
}
}
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 @@
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 @@
}
qd_field_iterator_t *reply_to = 0;
-
qd_composed_field_t *fld = 0;
// Start composing the message.
@@ -191,8 +192,6 @@
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 @@
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 @@
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);
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);
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 @@
qd_amqp_error_t status;
};
-ALLOC_DECLARE(qdr_query_t);
DEQ_DECLARE(qdr_query_t, qdr_query_list_t);
