Commit a6b70da9 authored by Mattias Jonsson's avatar Mattias Jonsson

Bug#11766249 bug#59316: PARTITIONING AND INDEX_MERGE MEMORY LEAK

When executing row-ordered-retrieval index merge,
the handler was cloned, but it used the wrong
memory root, so instead of allocating memory
on the thread/query's mem_root, it used the table's
mem_root, resulting in non released memory in the
table object, and was not freed until the table was
closed.

Solution was to ensure that memory used during cloning
of a handler was allocated from the correct memory root.

This was implemented by fixing handler::clone() to also
take a name argument, so it can be used with partitioning.
And in ha_partition only allocate the ha_partition's ref, and
call the original ha_partition partitions clone() and set at cloned
partitions.

Fix of .bzrignore on Windows with VS 2010
parent 2ab3b055
...@@ -37,7 +37,13 @@ ...@@ -37,7 +37,13 @@
*.user *.user
*.vcproj *.vcproj
*.vcproj.cmake *.vcproj.cmake
*.vcxproj
*.vcxproj.filters
*/*.dir/* */*.dir/*
*.dir
Debug
MySql.sdf
Win32
*/*_pure_*warnings */*_pure_*warnings
*/.deps */.deps
*/.libs/* */.libs/*
...@@ -46,6 +52,7 @@ ...@@ -46,6 +52,7 @@
*/minsizerel/* */minsizerel/*
*/release/* */release/*
*/relwithdebinfo/* */relwithdebinfo/*
RelWithDebInfo
*~ *~
.*.swp .*.swp
./CMakeCache.txt ./CMakeCache.txt
...@@ -607,6 +614,7 @@ include/mysql_h.ic ...@@ -607,6 +614,7 @@ include/mysql_h.ic
include/mysql_version.h include/mysql_version.h
include/mysqld_ername.h include/mysqld_ername.h
include/mysqld_error.h include/mysqld_error.h
include/mysqld_error.h.rule
include/openssl include/openssl
include/readline include/readline
include/readline/*.h include/readline/*.h
...@@ -1879,7 +1887,9 @@ scripts/mysql_find_rows ...@@ -1879,7 +1887,9 @@ scripts/mysql_find_rows
scripts/mysql_fix_extensions scripts/mysql_fix_extensions
scripts/mysql_fix_privilege_tables scripts/mysql_fix_privilege_tables
scripts/mysql_fix_privilege_tables.sql scripts/mysql_fix_privilege_tables.sql
scripts/mysql_fix_privilege_tables.sql.rule
scripts/mysql_fix_privilege_tables_sql.c scripts/mysql_fix_privilege_tables_sql.c
scripts/mysql_fix_privilege_tables_sql.c.rule
scripts/mysql_install_db scripts/mysql_install_db
scripts/mysql_secure_installation scripts/mysql_secure_installation
scripts/mysql_setpermission scripts/mysql_setpermission
...@@ -2116,6 +2126,7 @@ sql/handlerton.cc ...@@ -2116,6 +2126,7 @@ sql/handlerton.cc
sql/html sql/html
sql/latex sql/latex
sql/lex_hash.h sql/lex_hash.h
sql/lex_hash.h.rule
sql/link_sources sql/link_sources
sql/max/* sql/max/*
sql/message.h sql/message.h
...@@ -2147,6 +2158,7 @@ sql/sql_builtin.cc ...@@ -2147,6 +2158,7 @@ sql/sql_builtin.cc
sql/sql_select.cc.orig sql/sql_select.cc.orig
sql/sql_yacc.cc sql/sql_yacc.cc
sql/sql_yacc.h sql/sql_yacc.h
sql/sql_yacc.h.rule
sql/sql_yacc.output sql/sql_yacc.output
sql/sql_yacc.yy.orig sql/sql_yacc.yy.orig
sql/test_time sql/test_time
......
This diff is collapsed.
...@@ -133,6 +133,13 @@ class ha_partition :public handler ...@@ -133,6 +133,13 @@ class ha_partition :public handler
bool m_is_sub_partitioned; // Is subpartitioned bool m_is_sub_partitioned; // Is subpartitioned
bool m_ordered_scan_ongoing; bool m_ordered_scan_ongoing;
/*
If set, this object was created with ha_partition::clone and doesn't
"own" the m_part_info structure.
*/
ha_partition *m_is_clone_of;
MEM_ROOT *m_clone_mem_root;
/* /*
We keep track if all underlying handlers are MyISAM since MyISAM has a We keep track if all underlying handlers are MyISAM since MyISAM has a
great number of extra flags not needed by other handlers. great number of extra flags not needed by other handlers.
...@@ -169,11 +176,6 @@ class ha_partition :public handler ...@@ -169,11 +176,6 @@ class ha_partition :public handler
PARTITION_SHARE *share; /* Shared lock info */ PARTITION_SHARE *share; /* Shared lock info */
#endif #endif
/*
TRUE <=> this object was created with ha_partition::clone and doesn't
"own" the m_part_info structure.
*/
bool is_clone;
bool auto_increment_lock; /**< lock reading/updating auto_inc */ bool auto_increment_lock; /**< lock reading/updating auto_inc */
/** /**
Flag to keep the auto_increment lock through out the statement. Flag to keep the auto_increment lock through out the statement.
...@@ -186,7 +188,7 @@ class ha_partition :public handler ...@@ -186,7 +188,7 @@ class ha_partition :public handler
/** used for prediction of start_bulk_insert rows */ /** used for prediction of start_bulk_insert rows */
enum_monotonicity_info m_part_func_monotonicity_info; enum_monotonicity_info m_part_func_monotonicity_info;
public: public:
handler *clone(MEM_ROOT *mem_root); handler *clone(const char *name, MEM_ROOT *mem_root);
virtual void set_part_info(partition_info *part_info) virtual void set_part_info(partition_info *part_info)
{ {
m_part_info= part_info; m_part_info= part_info;
...@@ -205,6 +207,10 @@ class ha_partition :public handler ...@@ -205,6 +207,10 @@ class ha_partition :public handler
*/ */
ha_partition(handlerton *hton, TABLE_SHARE * table); ha_partition(handlerton *hton, TABLE_SHARE * table);
ha_partition(handlerton *hton, partition_info * part_info); ha_partition(handlerton *hton, partition_info * part_info);
ha_partition(handlerton *hton, TABLE_SHARE *share,
partition_info *part_info_arg,
ha_partition *clone_arg,
MEM_ROOT *clone_mem_root_arg);
~ha_partition(); ~ha_partition();
/* /*
A partition handler has no characteristics in itself. It only inherits A partition handler has no characteristics in itself. It only inherits
...@@ -275,7 +281,7 @@ class ha_partition :public handler ...@@ -275,7 +281,7 @@ class ha_partition :public handler
And one method to read it in. And one method to read it in.
*/ */
bool create_handler_file(const char *name); bool create_handler_file(const char *name);
bool get_from_handler_file(const char *name, MEM_ROOT *mem_root); bool get_from_handler_file(const char *name, MEM_ROOT *mem_root, bool clone);
bool new_handlers_from_part_info(MEM_ROOT *mem_root); bool new_handlers_from_part_info(MEM_ROOT *mem_root);
bool create_handlers(MEM_ROOT *mem_root); bool create_handlers(MEM_ROOT *mem_root);
void clear_handler_file(); void clear_handler_file();
......
...@@ -2037,9 +2037,9 @@ int ha_delete_table(THD *thd, handlerton *table_type, const char *path, ...@@ -2037,9 +2037,9 @@ int ha_delete_table(THD *thd, handlerton *table_type, const char *path,
/**************************************************************************** /****************************************************************************
** General handler functions ** General handler functions
****************************************************************************/ ****************************************************************************/
handler *handler::clone(MEM_ROOT *mem_root) handler *handler::clone(const char *name, MEM_ROOT *mem_root)
{ {
handler *new_handler= get_new_handler(table->s, mem_root, table->s->db_type()); handler *new_handler= get_new_handler(table->s, mem_root, ht);
/* /*
Allocate handler->ref here because otherwise ha_open will allocate it Allocate handler->ref here because otherwise ha_open will allocate it
on this->table->mem_root and we will not be able to reclaim that memory on this->table->mem_root and we will not be able to reclaim that memory
...@@ -2048,7 +2048,7 @@ handler *handler::clone(MEM_ROOT *mem_root) ...@@ -2048,7 +2048,7 @@ handler *handler::clone(MEM_ROOT *mem_root)
if (!(new_handler->ref= (uchar*) alloc_root(mem_root, ALIGN_SIZE(ref_length)*2))) if (!(new_handler->ref= (uchar*) alloc_root(mem_root, ALIGN_SIZE(ref_length)*2)))
return NULL; return NULL;
if (new_handler && !new_handler->ha_open(table, if (new_handler && !new_handler->ha_open(table,
table->s->normalized_path.str, name,
table->db_stat, table->db_stat,
HA_OPEN_IGNORE_IF_LOCKED)) HA_OPEN_IGNORE_IF_LOCKED))
return new_handler; return new_handler;
......
...@@ -1166,7 +1166,7 @@ class handler :public Sql_alloc ...@@ -1166,7 +1166,7 @@ class handler :public Sql_alloc
DBUG_ASSERT(locked == FALSE); DBUG_ASSERT(locked == FALSE);
/* TODO: DBUG_ASSERT(inited == NONE); */ /* TODO: DBUG_ASSERT(inited == NONE); */
} }
virtual handler *clone(MEM_ROOT *mem_root); virtual handler *clone(const char *name, MEM_ROOT *mem_root);
/** This is called after create to allow us to set up cached variables */ /** This is called after create to allow us to set up cached variables */
void init() void init()
{ {
......
...@@ -1335,7 +1335,7 @@ int QUICK_RANGE_SELECT::init_ror_merged_scan(bool reuse_handler) ...@@ -1335,7 +1335,7 @@ int QUICK_RANGE_SELECT::init_ror_merged_scan(bool reuse_handler)
} }
thd= head->in_use; thd= head->in_use;
if (!(file= head->file->clone(thd->mem_root))) if (!(file= head->file->clone(head->s->normalized_path.str, thd->mem_root)))
{ {
/* /*
Manually set the error flag. Note: there seems to be quite a few Manually set the error flag. Note: there seems to be quite a few
......
...@@ -142,11 +142,11 @@ int ha_heap::close(void) ...@@ -142,11 +142,11 @@ int ha_heap::close(void)
DESCRIPTION DESCRIPTION
Do same as default implementation but use file->s->name instead of Do same as default implementation but use file->s->name instead of
table->s->path. This is needed by Windows where the clone() call sees table->s->path. This is needed by Windows where the clone() call sees
'/'-delimited path in table->s->path, while ha_peap::open() was called '/'-delimited path in table->s->path, while ha_heap::open() was called
with '\'-delimited path. with '\'-delimited path.
*/ */
handler *ha_heap::clone(MEM_ROOT *mem_root) handler *ha_heap::clone(const char *name, MEM_ROOT *mem_root)
{ {
handler *new_handler= get_new_handler(table->s, mem_root, table->s->db_type()); handler *new_handler= get_new_handler(table->s, mem_root, table->s->db_type());
if (new_handler && !new_handler->ha_open(table, file->s->name, table->db_stat, if (new_handler && !new_handler->ha_open(table, file->s->name, table->db_stat,
......
...@@ -34,7 +34,7 @@ class ha_heap: public handler ...@@ -34,7 +34,7 @@ class ha_heap: public handler
public: public:
ha_heap(handlerton *hton, TABLE_SHARE *table); ha_heap(handlerton *hton, TABLE_SHARE *table);
~ha_heap() {} ~ha_heap() {}
handler *clone(MEM_ROOT *mem_root); handler *clone(const char *name, MEM_ROOT *mem_root);
const char *table_type() const const char *table_type() const
{ {
return (table->in_use->variables.sql_mode & MODE_MYSQL323) ? return (table->in_use->variables.sql_mode & MODE_MYSQL323) ?
......
...@@ -552,9 +552,10 @@ ha_myisam::ha_myisam(handlerton *hton, TABLE_SHARE *table_arg) ...@@ -552,9 +552,10 @@ ha_myisam::ha_myisam(handlerton *hton, TABLE_SHARE *table_arg)
can_enable_indexes(1) can_enable_indexes(1)
{} {}
handler *ha_myisam::clone(MEM_ROOT *mem_root) handler *ha_myisam::clone(const char *name, MEM_ROOT *mem_root)
{ {
ha_myisam *new_handler= static_cast <ha_myisam *>(handler::clone(mem_root)); ha_myisam *new_handler= static_cast <ha_myisam *>(handler::clone(name,
mem_root));
if (new_handler) if (new_handler)
new_handler->file->state= file->state; new_handler->file->state= file->state;
return new_handler; return new_handler;
......
...@@ -44,7 +44,7 @@ class ha_myisam: public handler ...@@ -44,7 +44,7 @@ class ha_myisam: public handler
public: public:
ha_myisam(handlerton *hton, TABLE_SHARE *table_arg); ha_myisam(handlerton *hton, TABLE_SHARE *table_arg);
~ha_myisam() {} ~ha_myisam() {}
handler *clone(MEM_ROOT *mem_root); handler *clone(const char *name, MEM_ROOT *mem_root);
const char *table_type() const { return "MyISAM"; } const char *table_type() const { return "MyISAM"; }
const char *index_type(uint key_number); const char *index_type(uint key_number);
const char **bas_ext() const; const char **bas_ext() const;
......
...@@ -459,8 +459,7 @@ int ha_myisammrg::open(const char *name, int mode __attribute__((unused)), ...@@ -459,8 +459,7 @@ int ha_myisammrg::open(const char *name, int mode __attribute__((unused)),
problem because all locking is handled by the original MERGE table problem because all locking is handled by the original MERGE table
from which this is cloned of. from which this is cloned of.
*/ */
if (!(file= myrg_open(table->s->normalized_path.str, table->db_stat, if (!(file= myrg_open(name, table->db_stat, HA_OPEN_IGNORE_IF_LOCKED)))
HA_OPEN_IGNORE_IF_LOCKED)))
{ {
DBUG_PRINT("error", ("my_errno %d", my_errno)); DBUG_PRINT("error", ("my_errno %d", my_errno));
DBUG_RETURN(my_errno ? my_errno : -1); DBUG_RETURN(my_errno ? my_errno : -1);
...@@ -484,7 +483,7 @@ int ha_myisammrg::open(const char *name, int mode __attribute__((unused)), ...@@ -484,7 +483,7 @@ int ha_myisammrg::open(const char *name, int mode __attribute__((unused)),
@return A cloned handler instance. @return A cloned handler instance.
*/ */
handler *ha_myisammrg::clone(MEM_ROOT *mem_root) handler *ha_myisammrg::clone(const char *name, MEM_ROOT *mem_root)
{ {
MYRG_TABLE *u_table,*newu_table; MYRG_TABLE *u_table,*newu_table;
ha_myisammrg *new_handler= ha_myisammrg *new_handler=
...@@ -505,8 +504,8 @@ handler *ha_myisammrg::clone(MEM_ROOT *mem_root) ...@@ -505,8 +504,8 @@ handler *ha_myisammrg::clone(MEM_ROOT *mem_root)
return NULL; return NULL;
} }
if (new_handler->ha_open(table, table->s->normalized_path.str, table->db_stat, if (new_handler->ha_open(table, name, table->db_stat,
HA_OPEN_IGNORE_IF_LOCKED)) HA_OPEN_IGNORE_IF_LOCKED))
{ {
delete new_handler; delete new_handler;
return NULL; return NULL;
......
...@@ -62,7 +62,7 @@ class ha_myisammrg: public handler ...@@ -62,7 +62,7 @@ class ha_myisammrg: public handler
int open(const char *name, int mode, uint test_if_locked); int open(const char *name, int mode, uint test_if_locked);
int attach_children(void); int attach_children(void);
int detach_children(void); int detach_children(void);
virtual handler *clone(MEM_ROOT *mem_root); virtual handler *clone(const char *name, MEM_ROOT *mem_root);
int close(void); int close(void);
int write_row(uchar * buf); int write_row(uchar * buf);
int update_row(const uchar * old_data, uchar * new_data); int update_row(const uchar * old_data, uchar * new_data);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment