From 08e6431c8c9c2b6174b9a46ec5f3f36c496f9a9d Mon Sep 17 00:00:00 2001 From: Monty Date: Fri, 24 Nov 2023 19:59:32 +0200 Subject: [PATCH] Fixed memory leak introduces by a fix for MDEV-29932 The leaks are all 40 bytes and happens in this call stack when running mtr vcol.vcol_syntax: alloc_root() ... Virtual_column_info::fix_and_check_exp() ... Delayed_insert::get_local_table() The problem was that one copied a MEM_ROOT from THD to a TABLE without taking into account that new blocks would be allocated through the TABLE memroot (and would thus be leaked). In general, one should NEVER copy MEM_ROOT from one object to another without clearing the copied memroot! Fixed by, at end of get_local_table(), copy all new allocated objects to client_thd->mem_root. Other things: - Removed references to MEM_ROOT::total_alloc that was wrongly left after a previous commit --- include/my_sys.h | 1 + mysys/my_alloc.c | 23 ++++++++++++++++++++++- sql/sql_insert.cc | 7 ++++++- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/include/my_sys.h b/include/my_sys.h index 3c44b6a0a80dd..82acdaa1a2aa1 100644 --- a/include/my_sys.h +++ b/include/my_sys.h @@ -892,6 +892,7 @@ extern void init_alloc_root(MEM_ROOT *mem_root, const char *name, extern void *alloc_root(MEM_ROOT *mem_root, size_t Size); extern void *multi_alloc_root(MEM_ROOT *mem_root, ...); extern void free_root(MEM_ROOT *root, myf MyFLAGS); +extern void move_root(MEM_ROOT *to, MEM_ROOT *from); extern void set_prealloc_root(MEM_ROOT *root, char *ptr); extern void reset_root_defaults(MEM_ROOT *mem_root, size_t block_size, size_t prealloc_size); diff --git a/mysys/my_alloc.c b/mysys/my_alloc.c index 983c4440243f6..d7151e5e8671f 100644 --- a/mysys/my_alloc.c +++ b/mysys/my_alloc.c @@ -199,7 +199,6 @@ void *alloc_root(MEM_ROOT *mem_root, size_t length) next->left= 0; next->size= length; mem_root->used= next; - mem_root->total_alloc+= length; DBUG_PRINT("exit",("ptr: %p", (((char*) next)+ ALIGN_SIZE(sizeof(USED_MEM))))); DBUG_RETURN((uchar*) (((char*) next)+ALIGN_SIZE(sizeof(USED_MEM)))); @@ -460,6 +459,28 @@ void set_prealloc_root(MEM_ROOT *root, char *ptr) } } +/* + Move allocated objects from one root to another. + + Notes: + We do not increase 'to->block_num' here as the variable isused to + increase block sizes in case of many allocations. This is special + case where this is not needed to take into account +*/ + +void move_root(MEM_ROOT *to, MEM_ROOT *from) +{ + USED_MEM *block, *next; + for (block= from->used; block ; block= next) + { + next= block->next; + block->next= to->used; + to->used= block; + } + from->used= 0; +} + + char *strdup_root(MEM_ROOT *root, const char *str) { diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 99592d79f16d0..933ce306861b4 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -2635,7 +2635,9 @@ TABLE *Delayed_insert::get_local_table(THD* client_thd) copy= new (copy_tmp) TABLE; *copy= *table; copy->vcol_refix_list.empty(); - copy->mem_root= *client_thd->mem_root; + init_alloc_root(©->mem_root, client_thd->mem_root->name, + client_thd->mem_root->block_size, + 0, MY_THREAD_SPECIFIC); /* We don't need to change the file handler here */ /* Assign the pointers for the field pointers array and the record. */ @@ -2729,11 +2731,14 @@ TABLE *Delayed_insert::get_local_table(THD* client_thd) bzero((char*) bitmap, share->column_bitmap_size * bitmaps_used); copy->read_set= ©->def_read_set; copy->write_set= ©->def_write_set; + move_root(client_thd->mem_root, ©->mem_root); + free_root(©->mem_root, 0); DBUG_RETURN(copy); /* Got fatal error */ error: + free_root(©->mem_root, 0); tables_in_use--; mysql_cond_signal(&cond); // Inform thread about abort DBUG_RETURN(0);