Skip to content

Commit

Permalink
Fixed memory leak introduces by a fix for MDEV-29932
Browse files Browse the repository at this point in the history
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
  • Loading branch information
montywi committed Nov 27, 2023
1 parent 8e96119 commit 08e6431
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 2 deletions.
1 change: 1 addition & 0 deletions include/my_sys.h
Expand Up @@ -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);
Expand Down
23 changes: 22 additions & 1 deletion mysys/my_alloc.c
Expand Up @@ -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))));
Expand Down Expand Up @@ -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)
{
Expand Down
7 changes: 6 additions & 1 deletion sql/sql_insert.cc
Expand Up @@ -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(&copy->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. */
Expand Down Expand Up @@ -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= &copy->def_read_set;
copy->write_set= &copy->def_write_set;
move_root(client_thd->mem_root, &copy->mem_root);
free_root(&copy->mem_root, 0);

DBUG_RETURN(copy);

/* Got fatal error */
error:
free_root(&copy->mem_root, 0);
tables_in_use--;
mysql_cond_signal(&cond); // Inform thread about abort
DBUG_RETURN(0);
Expand Down

0 comments on commit 08e6431

Please sign in to comment.