fix memory leaks and other problems found by safemalloc

client/mysql_upgrade.c:
  missing DBUG_RETURN
client/mysqladmin.cc:
  client plugin memory wasn't freed
client/mysqlcheck.c:
  client plugin memory, defaults, a result set, a command-line option value were not freed.
  missing DBUG_RETURN.
client/mysqldump.c:
  client plugin memory wasn't freed
client/mysqlslap.c:
  client plugin memory wasn't freed
client/mysqltest.cc:
  hopeless. cannot be fixed.
mysql-test/valgrind.supp:
  Bug#56666 is now fixed.
mysys/array.c:
  really, don't allocate if the caller didn't ask to.
mysys/my_init.c:
  safemalloc checks must be done at the very end
mysys/my_thr_init.c:
  not needed anymore
sql-common/client.c:
  memory leak
sql/log.cc:
  log_file was not closed, memory leak.
sql/mysqld.cc:
  fix bug#56666 (causing many P_S related memory leaks).
  close_active_mi() not called for --bootstrap, memory leak.
sql/sql_lex.cc:
  redo Lex->mi handling
sql/sql_lex.h:
  redo Lex->mi handling
sql/sql_plugin.cc:
  plugins having PLUGIN_VAR_MEMALLOC string variables have this variables allocated in every THD.
  The memory was freed in ~THD but only if plugin was still active. If plugin was unloaded the
  variable was not found and the memory was lost. By loading and unloading plugins an arbitrary
  amount of memory can be lost.
sql/sql_repl.cc:
  redo Lex->mi handling
sql/sql_yacc.yy:
  completely wrong handling of Lex->mi - run-time memory leak, by repeating the statement
  arbitrary amount of memory can be lost.
  
  Lex->mi.repl_ignore_server_ids_opt was allocated when parsing CHANGE MASTER,
  and freed after executing the statement. if parser failed on syntax (or another)
  error the statement was never executed. Lex->mi was simply bzero-ed for the next
  CHANGE MASTER  statement.
sql/table.cc:
  didn't compile
storage/perfschema/pfs_lock.h:
  Bug#56666 is fixed
This commit is contained in:
Sergei Golubchik 2011-07-10 20:09:17 +02:00
parent 172f5e28ba
commit 49501b4ccb
21 changed files with 103 additions and 179 deletions

View file

@ -833,7 +833,7 @@ static int run_sql_fix_privilege_tables(void)
}
dynstr_free(&ds_result);
return found_real_errors;
DBUG_RETURN(found_real_errors);
}

View file

@ -462,6 +462,7 @@ int main(int argc,char *argv[])
} /* got connection */
mysql_close(&mysql);
mysql_library_end();
my_free(opt_password);
my_free(user);
#ifdef HAVE_SMEM

View file

@ -126,7 +126,7 @@ static struct my_option my_long_options[] =
{"help", '?', "Display this help message and exit.", 0, 0, 0, GET_NO_ARG,
NO_ARG, 0, 0, 0, 0, 0, 0},
{"host",'h', "Connect to host.", &current_host,
&current_host, 0, GET_STR_ALLOC, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
&current_host, 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
{"medium-check", 'm',
"Faster than extended-check, but only finds 99.99 percent of all errors. Should be good enough for most cases.",
0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0},
@ -351,8 +351,7 @@ static int get_options(int *argc, char ***argv)
exit(0);
}
if ((ho_error= load_defaults("my", load_default_groups, argc, argv)) ||
(ho_error=handle_options(argc, argv, my_long_options, get_one_option)))
if ((ho_error=handle_options(argc, argv, my_long_options, get_one_option)))
exit(ho_error);
if (!what_to_do)
@ -432,6 +431,7 @@ static int process_all_databases()
if (process_one_db(row[0]))
result = 1;
}
mysql_free_result(tableres);
return result;
}
/* process_all_databases */
@ -879,10 +879,10 @@ static int dbConnect(char *host, char *user, char *passwd)
NULL, opt_mysql_port, opt_mysql_unix_port, 0)))
{
DBerror(&mysql_connection, "when trying to connect");
return 1;
DBUG_RETURN(1);
}
mysql_connection.reconnect= 1;
return 0;
DBUG_RETURN(0);
} /* dbConnect */
@ -918,34 +918,36 @@ static void safe_exit(int error)
int main(int argc, char **argv)
{
int ret= EX_USAGE;
char **defaults_argv;
MY_INIT(argv[0]);
mysql_library_init(-1, 0, 0);
/*
** Check out the args
*/
if (get_options(&argc, &argv))
{
my_end(my_end_arg);
exit(EX_USAGE);
}
if (dbConnect(current_host, current_user, opt_password))
exit(EX_MYSQLERR);
if (load_defaults("my", load_default_groups, &argc, &argv))
goto end1;
defaults_argv= argv;
if (get_options(&argc, &argv))
goto end1;
ret= EX_MYSQLERR;
if (dbConnect(current_host, current_user, opt_password))
goto end1;
ret= 1;
if (!opt_write_binlog)
{
if (disable_binlog())
{
first_error= 1;
goto end;
}
}
if (opt_auto_repair &&
(my_init_dynamic_array(&tables4repair, sizeof(char)*(NAME_LEN*2+2),16,64) ||
my_init_dynamic_array(&tables4rebuild, sizeof(char)*(NAME_LEN*2+2),16,64)))
{
first_error = 1;
goto end;
}
if (opt_alldbs)
process_all_databases();
@ -970,6 +972,8 @@ int main(int argc, char **argv)
for (i = 0; i < tables4rebuild.elements ; i++)
rebuild_table((char*) dynamic_array_ptr(&tables4rebuild, i));
}
ret= test(first_error);
end:
dbDisconnect(current_host);
if (opt_auto_repair)
@ -977,10 +981,13 @@ int main(int argc, char **argv)
delete_dynamic(&tables4repair);
delete_dynamic(&tables4rebuild);
}
end1:
my_free(opt_password);
#ifdef HAVE_SMEM
my_free(shared_memory_base_name);
#endif
mysql_library_end();
free_defaults(defaults_argv);
my_end(my_end_arg);
return(first_error!=0);
return ret;
} /* main */

View file

@ -1423,6 +1423,7 @@ static void free_resources()
dynstr_free(&insert_pat);
if (defaults_argv)
free_defaults(defaults_argv);
mysql_library_end();
my_end(my_end_arg);
}

View file

@ -421,6 +421,7 @@ int main(int argc, char **argv)
my_free(shared_memory_base_name);
#endif
free_defaults(defaults_argv);
mysql_library_end();
my_end(my_end_arg);
return 0;

View file

@ -1319,6 +1319,15 @@ void free_used_memory()
static void cleanup_and_exit(int exit_code)
{
free_used_memory();
/*
mysqltest is fundamentally written in a way that makes impossible
to free all memory before exit (consider memory allocated
for frame local DYNAMIC_STRING's and die() invoked down the stack.
We close stderr here to stop unavoidable safemalloc reports
from polluting the output.
*/
fclose(stderr);
my_end(my_end_arg);
if (!silent) {

View file

@ -1093,78 +1093,6 @@
fun:buf_buddy_relocate
}
#
# See related Bug#56666
# Race condition between the server main thread and the kill server thread.
#
# Because of this race condition, the call to shutdown_performance_schema()
# was commented in sql/mysqld.cc, causing the reported leaks.
#
{
missing shutdown_performance_schema 1
Memcheck:Leak
fun:malloc
fun:_Z10pfs_mallocmi
}
{
missing shutdown_performance_schema 2
Memcheck:Leak
fun:malloc
fun:my_malloc
fun:_lf_alloc_new
fun:lf_hash_insert
}
#
# Note that initialize_bucket() is reccursive,
# can't provide more stack context.
#
{
missing shutdown_performance_schema 3
Memcheck:Leak
fun:malloc
fun:my_malloc
fun:initialize_bucket
}
{
missing shutdown_performance_schema 4
Memcheck:Leak
fun:malloc
fun:my_malloc
fun:_lf_dynarray_lvalue
fun:_lf_pinbox_get_pins
}
{
missing shutdown_performance_schema 5
Memcheck:Leak
fun:malloc
fun:my_malloc
fun:_lf_dynarray_lvalue
fun:lf_hash_insert
}
{
missing shutdown_performance_schema 6
Memcheck:Leak
fun:malloc
fun:my_malloc
fun:_lf_dynarray_lvalue
fun:lf_hash_delete
}
{
missing shutdown_performance_schema 7
Memcheck:Leak
fun:malloc
fun:my_malloc
fun:_lf_dynarray_lvalue
fun:lf_hash_search
}
{
Bug 59874 Valgrind warning in InnoDB compression code
Memcheck:Cond

View file

@ -61,7 +61,8 @@ my_bool init_dynamic_array2(DYNAMIC_ARRAY *array, uint element_size,
Since the dynamic array is usable even if allocation fails here malloc
should not throw an error
*/
if (!(array->buffer= (uchar*) my_malloc(element_size*init_alloc, MYF(0))))
if (init_alloc &&
!(array->buffer= (uchar*) my_malloc(element_size*init_alloc, MYF(0))))
array->max_element=0;
DBUG_RETURN(FALSE);
}

View file

@ -201,11 +201,6 @@ Voluntary context switches %ld, Involuntary context switches %ld\n",
#endif
}
if (!(infoflag & MY_DONT_FREE_DBUG))
{
DBUG_END(); /* Must be done before my_thread_end */
}
my_thread_end();
my_thread_global_end();
my_mutex_end();
@ -223,6 +218,9 @@ Voluntary context switches %ld, Involuntary context switches %ld\n",
WSACleanup();
#endif /* __WIN__ */
if (!(infoflag & MY_DONT_FREE_DBUG))
DBUG_END(); /* Must be done as late as possible */
my_init_done=0;
} /* my_end */

View file

@ -366,10 +366,7 @@ void my_thread_end(void)
tmp->dbug=0;
}
#endif
#if !defined(__bsdi__) && !defined(__OpenBSD__)
/* bsdi and openbsd 3.5 dumps core here */
mysql_cond_destroy(&tmp->suspend);
#endif
mysql_mutex_destroy(&tmp->mutex);
/*

View file

@ -2189,6 +2189,7 @@ mysql_autodetect_character_set(MYSQL *mysql)
}
#endif
my_free(mysql->options.charset_name);
if (!(mysql->options.charset_name= my_strdup(csname, MYF(MY_WME))))
return 1;
return 0;

View file

@ -3170,11 +3170,7 @@ To turn it on again: fix the cause, \
shutdown the MySQL server and restart it.", name, errno);
if (file >= 0)
mysql_file_close(file, MYF(0));
end_io_cache(&log_file);
end_io_cache(&index_file);
my_free(name);
name= NULL;
log_state= LOG_CLOSED;
close(LOG_CLOSE_INDEX);
DBUG_RETURN(1);
}

View file

@ -336,8 +336,7 @@ static PSI_rwlock_key key_rwlock_openssl;
static bool lower_case_table_names_used= 0;
static bool max_long_data_size_used= false;
static bool volatile select_thread_in_use, signal_thread_in_use;
/* See Bug#56666 and Bug#56760 */;
volatile bool ready_to_exit;
static volatile bool ready_to_exit;
static my_bool opt_debugging= 0, opt_external_locking= 0, opt_console= 0;
static my_bool opt_short_log_format= 0;
static my_bool opt_sync= 0;
@ -1185,7 +1184,6 @@ static void close_connections(void)
}
mysql_mutex_unlock(&LOCK_thread_count);
close_active_mi();
DBUG_PRINT("quit",("close_connections thread"));
DBUG_VOID_RETURN;
}
@ -1445,13 +1443,9 @@ static void mysqld_exit(int exit_code)
mysql_audit_finalize();
clean_up_mutexes();
clean_up_error_log_mutex();
#ifdef WITH_PERFSCHEMA_STORAGE_ENGINE
/*
Bug#56666 needs to be fixed before calling:
shutdown_performance_schema();
*/
#endif
my_end(opt_endinfo ? MY_CHECK_ERROR | MY_GIVE_INFO : 0);
my_end((opt_endinfo ? MY_CHECK_ERROR | MY_GIVE_INFO : 0) | MY_DONT_FREE_DBUG);
shutdown_performance_schema(); // we do it as late as possible
DBUG_END(); // but this - even later
exit(exit_code); /* purecov: inspected */
}
@ -1463,6 +1457,7 @@ void clean_up(bool print_message)
if (cleanup_done++)
return; /* purecov: inspected */
close_active_mi();
stop_handle_manager();
release_ddl_log();
@ -1553,6 +1548,7 @@ void clean_up(bool print_message)
DBUG_PRINT("quit", ("Error messages freed"));
/* Tell main we are ready */
logger.cleanup_end();
sys_var_end();
my_atomic_rwlock_destroy(&global_query_id_lock);
my_atomic_rwlock_destroy(&thread_running_lock);
mysql_mutex_lock(&LOCK_thread_count);
@ -1561,7 +1557,6 @@ void clean_up(bool print_message)
/* do the broadcast inside the lock to ensure that my_end() is not called */
mysql_cond_broadcast(&COND_thread_count);
mysql_mutex_unlock(&LOCK_thread_count);
sys_var_end();
/*
The following lines may never be executed as the main thread may have
@ -1586,11 +1581,8 @@ static void wait_for_signal_thread_to_end()
*/
for (i= 0 ; i < 100 && signal_thread_in_use; i++)
{
if (pthread_kill(signal_thread, MYSQL_KILL_SIGNAL) != ESRCH)
{
fprintf(stderr, "signal thread appears to be dead\n");
if (pthread_kill(signal_thread, MYSQL_KILL_SIGNAL) == ESRCH)
break;
}
my_sleep(100); // Give it time to die
}
}
@ -4826,7 +4818,6 @@ int mysqld_main(int argc, char **argv)
CloseHandle(hEventShutdown);
}
#endif
clean_up(1);
mysqld_exit(0);
}

View file

@ -237,6 +237,8 @@ extern char *master_ssl_cipher, *master_ssl_key;
extern I_List<THD> threads;
#else
#define close_active_mi() /* no-op */
#endif /* HAVE_REPLICATION */
/* masks for start/stop operations on io and sql slave threads */

View file

@ -534,6 +534,8 @@ void lex_end(LEX *lex)
delete lex->sphead;
lex->sphead= NULL;
lex->mi.reset();
DBUG_VOID_RETURN;
}
@ -2505,6 +2507,7 @@ LEX::LEX()
INITIAL_LEX_PLUGIN_LIST_SIZE,
INITIAL_LEX_PLUGIN_LIST_SIZE);
reset_query_tables_list(TRUE);
mi.init();
}

View file

@ -278,24 +278,41 @@ typedef struct st_lex_server_options
would better be renamed to st_lex_replication_info). Some fields,
e.g., delay, are saved in Relay_log_info, not in Master_info.
*/
typedef struct st_lex_master_info
struct LEX_MASTER_INFO
{
DYNAMIC_ARRAY repl_ignore_server_ids;
char *host, *user, *password, *log_file_name;
char *ssl_key, *ssl_cert, *ssl_ca, *ssl_capath, *ssl_cipher;
char *relay_log_name;
ulonglong pos;
ulong relay_log_pos;
ulong server_id;
uint port, connect_retry;
float heartbeat_period;
ulonglong pos;
ulong server_id;
/*
Enum is used for making it possible to detect if the user
changed variable or if it should be left at old value
*/
enum {LEX_MI_UNCHANGED, LEX_MI_DISABLE, LEX_MI_ENABLE}
ssl, ssl_verify_server_cert, heartbeat_opt, repl_ignore_server_ids_opt;
char *ssl_key, *ssl_cert, *ssl_ca, *ssl_capath, *ssl_cipher;
char *relay_log_name;
ulong relay_log_pos;
DYNAMIC_ARRAY repl_ignore_server_ids;
} LEX_MASTER_INFO;
void init()
{
bzero(this, sizeof(*this));
my_init_dynamic_array(&repl_ignore_server_ids,
sizeof(::server_id), 0, 16);
}
void reset()
{
delete_dynamic(&repl_ignore_server_ids);
host= user= password= log_file_name= ssl_key= ssl_cert= ssl_ca=
ssl_capath= ssl_cipher= relay_log_name= 0;
pos= relay_log_pos= server_id= port= connect_retry= 0;
heartbeat_period= 0;
ssl= ssl_verify_server_cert= heartbeat_opt=
repl_ignore_server_ids_opt= LEX_MI_UNCHANGED;
}
};
enum sub_select_type

View file

@ -1057,10 +1057,6 @@ static bool plugin_add(MEM_ROOT *tmp_root,
mysql_del_sys_var_chain(tmp.system_vars);
restore_pluginvar_names(tmp.system_vars);
goto err;
/* plugin was disabled */
plugin_dl_del(dl);
DBUG_RETURN(FALSE);
}
}
report_error(report, ER_CANT_FIND_DL_ENTRY, name->str);
@ -1899,7 +1895,8 @@ void plugin_shutdown(void)
if (plugins[i]->ref_count)
sql_print_error("Plugin '%s' has ref_count=%d after shutdown.",
plugins[i]->name.str, plugins[i]->ref_count);
if (plugins[i]->state & PLUGIN_IS_UNINITIALIZED)
if (plugins[i]->state & PLUGIN_IS_UNINITIALIZED ||
plugins[i]->state & PLUGIN_IS_DISABLED)
plugin_del(plugins[i]);
}
@ -2221,6 +2218,7 @@ err:
#undef MYSQL_SYSVAR_NAME
#define MYSQL_SYSVAR_NAME(name) name
#define PLUGIN_VAR_TYPEMASK 0x007f
#define PLUGIN_VAR_BOOKMARK_KEY (PLUGIN_VAR_TYPEMASK | PLUGIN_VAR_MEMALLOC)
#define EXTRA_OPTIONS 3 /* options for: 'foo', 'plugin-foo' and NULL */
@ -2574,7 +2572,7 @@ static st_bookmark *find_bookmark(const char *plugin, const char *name,
else
memcpy(varname + 1, name, namelen + 1);
varname[0]= flags & PLUGIN_VAR_TYPEMASK;
varname[0]= flags & PLUGIN_VAR_BOOKMARK_KEY;
result= (st_bookmark*) my_hash_search(&bookmark_hash,
(const uchar*) varname, length - 1);
@ -2632,7 +2630,7 @@ static st_bookmark *register_var(const char *plugin, const char *name,
{
result= (st_bookmark*) alloc_root(&plugin_mem_root,
sizeof(struct st_bookmark) + length-1);
varname[0]= flags & PLUGIN_VAR_TYPEMASK;
varname[0]= flags & PLUGIN_VAR_BOOKMARK_KEY;
memcpy(result->key, varname, length);
result->name_len= length - 2;
result->offset= -1;
@ -2748,10 +2746,12 @@ static uchar *intern_sys_var_ptr(THD* thd, int offset, bool global_lock)
sys_var *var;
st_bookmark *v= (st_bookmark*) my_hash_element(&bookmark_hash,idx);
if (v->version <= thd->variables.dynamic_variables_version ||
!(var= intern_find_sys_var(v->key + 1, v->name_len)) ||
if (v->version <= thd->variables.dynamic_variables_version)
continue; /* already in thd->variables */
if (!(var= intern_find_sys_var(v->key + 1, v->name_len)) ||
!(pi= var->cast_pluginvar()) ||
v->key[0] != (pi->plugin_var->flags & PLUGIN_VAR_TYPEMASK))
v->key[0] != (pi->plugin_var->flags & PLUGIN_VAR_BOOKMARK_KEY))
continue;
/* Here we do anything special that may be required of the data types */
@ -2870,27 +2870,22 @@ static void unlock_variables(THD *thd, struct system_variables *vars)
static void cleanup_variables(THD *thd, struct system_variables *vars)
{
st_bookmark *v;
sys_var_pluginvar *pivar;
sys_var *var;
int flags;
uint idx;
mysql_rwlock_rdlock(&LOCK_system_variables_hash);
for (idx= 0; idx < bookmark_hash.records; idx++)
{
v= (st_bookmark*) my_hash_element(&bookmark_hash, idx);
if (v->version > vars->dynamic_variables_version ||
!(var= intern_find_sys_var(v->key + 1, v->name_len)) ||
!(pivar= var->cast_pluginvar()) ||
v->key[0] != (pivar->plugin_var->flags & PLUGIN_VAR_TYPEMASK))
continue;
flags= pivar->plugin_var->flags;
if (v->version > vars->dynamic_variables_version)
continue; /* not in vars */
if ((flags & PLUGIN_VAR_TYPEMASK) == PLUGIN_VAR_STR &&
flags & PLUGIN_VAR_THDLOCAL && flags & PLUGIN_VAR_MEMALLOC)
DBUG_ASSERT((uint)v->offset <= vars->dynamic_variables_head);
if ((v->key[0] & PLUGIN_VAR_TYPEMASK) == PLUGIN_VAR_STR &&
v->key[0] & PLUGIN_VAR_MEMALLOC)
{
char **ptr= (char**) pivar->real_value_ptr(thd, OPT_SESSION);
char **ptr= (char**)(vars->dynamic_variables_ptr + v->offset);
my_free(*ptr);
*ptr= NULL;
}

View file

@ -1665,7 +1665,6 @@ err:
thd_proc_info(thd, 0);
if (ret == FALSE)
my_ok(thd);
delete_dynamic(&lex_mi->repl_ignore_server_ids); //freeing of parser-time alloc
DBUG_RETURN(ret);
}

View file

@ -1879,15 +1879,7 @@ help:
change:
CHANGE MASTER_SYM TO_SYM
{
LEX *lex = Lex;
lex->sql_command = SQLCOM_CHANGE_MASTER;
bzero((char*) &lex->mi, sizeof(lex->mi));
/*
resetting flags that can left from the previous CHANGE MASTER
*/
lex->mi.repl_ignore_server_ids_opt= LEX_MASTER_INFO::LEX_MI_UNCHANGED;
my_init_dynamic_array(&Lex->mi.repl_ignore_server_ids,
sizeof(::server_id), 16, 16);
Lex->sql_command = SQLCOM_CHANGE_MASTER;
}
master_defs
{}
@ -7008,8 +7000,6 @@ slave:
LEX *lex=Lex;
lex->sql_command = SQLCOM_SLAVE_START;
lex->type = 0;
/* We'll use mi structure for UNTIL options */
bzero((char*) &lex->mi, sizeof(lex->mi));
/* If you change this code don't forget to update SLAVE START too */
}
slave_until
@ -7026,8 +7016,6 @@ slave:
LEX *lex=Lex;
lex->sql_command = SQLCOM_SLAVE_START;
lex->type = 0;
/* We'll use mi structure for UNTIL options */
bzero((char*) &lex->mi, sizeof(lex->mi));
}
slave_until
{}

View file

@ -3748,7 +3748,7 @@ void TABLE::init(THD *thd, TABLE_LIST *tl)
DBUG_ASSERT(key_read == 0);
/* mark the record[0] uninitialized */
TRASH(table->record[0], table->s->reclength);
TRASH(record[0], s->reclength);
/* Tables may be reused in a sub statement. */
DBUG_ASSERT(!file->extra(HA_EXTRA_IS_ATTACHED_CHILDREN));

View file

@ -135,25 +135,14 @@ struct pfs_lock
*/
void allocated_to_free(void)
{
#ifndef DBUG_OFF
extern volatile bool ready_to_exit;
#endif
/*
If this record is not in the ALLOCATED state and the caller is trying
to free it, this is a bug: the caller is confused,
and potentially damaging data owned by another thread or object.
The correct assert to use here to guarantee data integrity is simply:
DBUG_ASSERT(m_state == PFS_LOCK_ALLOCATED);
Now, because of Bug#56666 (Race condition between the server main thread
and the kill server thread), this assert actually fails during shutdown,
and the failure is legitimate, on concurrent calls to mysql_*_destroy(),
when destroying the instrumentation of an object ... twice.
During shutdown this has no consequences for the performance schema,
so the assert is relaxed with the "|| ready_to_exit" condition as a work
around until Bug#56666 is fixed.
*/
DBUG_ASSERT((m_state == PFS_LOCK_ALLOCATED) || ready_to_exit);
DBUG_ASSERT(m_state == PFS_LOCK_ALLOCATED);
PFS_atomic::store_32(&m_state, PFS_LOCK_FREE);
}