From bee7cc20026e679eea045a878547750639d46b13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 13 Sep 2018 13:56:19 +0300 Subject: [PATCH] MXS-2046: Fix additional memory leaks A set of memory leaks were revealed by Valgrind. --- server/modules/routing/avrorouter/avro_file.c | 1 + .../modules/routing/avrorouter/avro_schema.c | 22 ------------------- server/modules/routing/binlogrouter/blr.c | 6 ++++- .../modules/routing/binlogrouter/blr_file.c | 4 ++++ .../modules/routing/binlogrouter/blr_master.c | 21 ++++++++++++------ .../modules/routing/binlogrouter/blr_slave.c | 13 +++++++++-- 6 files changed, 35 insertions(+), 32 deletions(-) diff --git a/server/modules/routing/avrorouter/avro_file.c b/server/modules/routing/avrorouter/avro_file.c index d0772aeb9..8572b5248 100644 --- a/server/modules/routing/avrorouter/avro_file.c +++ b/server/modules/routing/avrorouter/avro_file.c @@ -697,6 +697,7 @@ avro_binlog_end_t avro_read_all_events(AVRO_INSTANCE *router) MXS_INFO("Annotate_rows_event: %.*s", hdr.event_size - BINLOG_EVENT_HDR_LEN, ptr); pos += original_size; router->current_pos = pos; + gwbuf_free(result); continue; } else if (hdr.event_type == TABLE_MAP_EVENT) diff --git a/server/modules/routing/avrorouter/avro_schema.c b/server/modules/routing/avrorouter/avro_schema.c index c6a29279b..e21259122 100644 --- a/server/modules/routing/avrorouter/avro_schema.c +++ b/server/modules/routing/avrorouter/avro_schema.c @@ -194,28 +194,6 @@ bool json_extract_field_names(const char* filename, TABLE_CREATE *table) if (json_is_object(val)) { - json_t* value; - - if ((value = json_object_get(val, "real_type")) && json_is_string(value)) - { - table->column_types[columns] = MXS_STRDUP_A(json_string_value(value)); - } - else - { - table->column_types[columns] = MXS_STRDUP_A("unknown"); - MXS_WARNING("No \"real_type\" value defined. Treating as unknown type field."); - } - - if ((value = json_object_get(val, "length")) && json_is_integer(value)) - { - table->column_lengths[columns] = json_integer_value(value); - } - else - { - table->column_lengths[columns] = -1; - MXS_WARNING("No \"length\" value defined. Treating as default length field."); - } - json_t *name = json_object_get(val, "name"); if (name && json_is_string(name)) diff --git a/server/modules/routing/binlogrouter/blr.c b/server/modules/routing/binlogrouter/blr.c index a46a3226e..144739ff6 100644 --- a/server/modules/routing/binlogrouter/blr.c +++ b/server/modules/routing/binlogrouter/blr.c @@ -1225,6 +1225,7 @@ newSession(MXS_ROUTER *instance, MXS_SESSION *session) slave->gtid_maps = NULL; memset(&slave->f_info, 0, sizeof (MARIADB_GTID_INFO)); slave->annotate_rows = false; + slave->warning_msg = NULL; /** * Add this session to the list of active sessions. @@ -1312,6 +1313,7 @@ static void freeSession(MXS_ROUTER* router_instance, { MXS_FREE(slave->mariadb_gtid); } + MXS_FREE(slave->warning_msg); MXS_FREE(slave); } @@ -1414,7 +1416,9 @@ routeQuery(MXS_ROUTER *instance, MXS_ROUTER_SESSION *router_session, GWBUF *queu ROUTER_INSTANCE *router = (ROUTER_INSTANCE *)instance; ROUTER_SLAVE *slave = (ROUTER_SLAVE *)router_session; - return blr_slave_request(router, slave, queue); + int rc = blr_slave_request(router, slave, queue); + gwbuf_free(queue); + return rc; } static char *event_names[] = diff --git a/server/modules/routing/binlogrouter/blr_file.c b/server/modules/routing/binlogrouter/blr_file.c index 81cc564c2..d2865032f 100644 --- a/server/modules/routing/binlogrouter/blr_file.c +++ b/server/modules/routing/binlogrouter/blr_file.c @@ -1404,6 +1404,7 @@ blr_read_binlog(ROUTER_INSTANCE *router, void blr_close_binlog(ROUTER_INSTANCE *router, BLFILE *file) { + ss_dassert(file); spinlock_acquire(&router->fileslock); file->refcnt--; if (file->refcnt == 0) @@ -4055,6 +4056,7 @@ bool blr_save_mariadb_gtid(ROUTER_INSTANCE *inst) { if (sql_ret == SQLITE_CONSTRAINT) { + sqlite3_free(errmsg); /* Prepare UPDATE SQL */ snprintf(sql_stmt, GTID_SQL_BUFFER_SIZE, @@ -4102,6 +4104,8 @@ bool blr_save_mariadb_gtid(ROUTER_INSTANCE *inst) } } + sqlite3_free(errmsg); + MXS_DEBUG("Saved/udated MariaDB GTID '%s', %s:%lu,%lu, SQL [%s]", gtid_info.gtid, inst->binlog_name, diff --git a/server/modules/routing/binlogrouter/blr_master.c b/server/modules/routing/binlogrouter/blr_master.c index 6a57036ea..ce5ec6ff8 100644 --- a/server/modules/routing/binlogrouter/blr_master.c +++ b/server/modules/routing/binlogrouter/blr_master.c @@ -153,6 +153,8 @@ static void blr_start_master(void* data) if (router->client) { + MXS_FREE(router->client->data); + router->client->data = NULL; dcb_close(router->client); router->client = NULL; } @@ -655,6 +657,7 @@ blr_master_response(ROUTER_INSTANCE *router, GWBUF *buf) router->active_logs = 0; spinlock_release(&router->lock); atomic_add(&router->handling_threads, -1); + gwbuf_free(buf); } /** @@ -2010,13 +2013,13 @@ blr_check_heartbeat(ROUTER_INSTANCE *router) static void blr_log_identity(ROUTER_INSTANCE *router) { - char *master_uuid; - char *master_hostname; - char *master_version; + char *master_uuid = NULL; + char *master_hostname = NULL; + char *master_version = NULL; if (router->set_master_version) { - master_version = router->set_master_version; + master_version = MXS_STRDUP(router->set_master_version); } else { @@ -2025,16 +2028,16 @@ static void blr_log_identity(ROUTER_INSTANCE *router) if (router->set_master_hostname) { - master_hostname = router->set_master_hostname; + master_hostname = MXS_STRDUP(router->set_master_hostname); } else { master_hostname = blr_extract_column(router->saved_master.selecthostname, 1); } - if (router->set_master_uuid) + if (router->set_master_uuid && router->master_uuid) { - master_uuid = router->master_uuid; + master_uuid = MXS_STRDUP(router->master_uuid); } else { @@ -2074,6 +2077,10 @@ static void blr_log_identity(ROUTER_INSTANCE *router) (master_hostname == NULL ? "not available" : master_hostname), (master_version == NULL ? "not available" : master_version)); } + + MXS_FREE(master_version); + MXS_FREE(master_hostname); + MXS_FREE(master_uuid); } /** diff --git a/server/modules/routing/binlogrouter/blr_slave.c b/server/modules/routing/binlogrouter/blr_slave.c index 15f8b1d5c..e351ec951 100644 --- a/server/modules/routing/binlogrouter/blr_slave.c +++ b/server/modules/routing/binlogrouter/blr_slave.c @@ -417,7 +417,7 @@ blr_slave_request(ROUTER_INSTANCE *router, ROUTER_SLAVE *slave, GWBUF *queue) { MXS_ERROR("Invalid slave state machine state (%d) for binlog router.", slave->state); - gwbuf_consume(queue, gwbuf_length(queue)); + gwbuf_free(queue); return 0; } @@ -495,7 +495,6 @@ blr_slave_request(ROUTER_INSTANCE *router, ROUTER_SLAVE *slave, GWBUF *queue) break; } - gwbuf_free(queue); return rv; } @@ -2159,6 +2158,7 @@ blr_slave_binlog_dump(ROUTER_INSTANCE *router, ROUTER_SLAVE *slave, GWBUF *queue blr_slave_abort_dump_request(slave, errmsg); slave->state = BLRS_ERRORED; dcb_close(slave->dcb); + gwbuf_free(fde); return 1; } } @@ -2202,6 +2202,7 @@ blr_slave_binlog_dump(ROUTER_INSTANCE *router, ROUTER_SLAVE *slave, GWBUF *queue blr_slave_abort_dump_request(slave, errmsg); slave->state = BLRS_ERRORED; dcb_close(slave->dcb); + gwbuf_free(fde); return 1; } slave->lastEventReceived = MARIADB10_GTID_GTID_LIST_EVENT; @@ -2222,6 +2223,7 @@ blr_slave_binlog_dump(ROUTER_INSTANCE *router, ROUTER_SLAVE *slave, GWBUF *queue /* Force the slave to call catchup routine */ poll_fake_write_event(slave->dcb); + gwbuf_free(fde); return 1; } @@ -2787,6 +2789,7 @@ blr_slave_catchup(ROUTER_INSTANCE *router, ROUTER_SLAVE *slave, bool large) slave->stats.n_events - events_before, router->current_safe_event, read_errmsg); + hdr.ok = SLAVE_POS_READ_OK; } } @@ -3060,6 +3063,9 @@ blr_slave_catchup(ROUTER_INSTANCE *router, ROUTER_SLAVE *slave, bool large) * Fake rotate just written to client, * no need to call poll_fake_write_event() */ + + // blr_slave_fake_rotate closes the file on success + file = NULL; } else { @@ -6485,6 +6491,7 @@ blr_slave_read_ste(ROUTER_INSTANCE *router, if (!new_encryption_ctx) { + gwbuf_free(record); return 0; } record_ptr += BINLOG_EVENT_HDR_LEN; @@ -6521,9 +6528,11 @@ blr_slave_read_ste(ROUTER_INSTANCE *router, * Note: if the requested pos is equal to START_ENCRYPTION_EVENT pos * the event will be skipped by blr_read_binlog() routine */ + gwbuf_free(record); return 1; } + gwbuf_free(record); return 0; }