BUG#48993: valgrind errors in mysqlbinlog
I found three issues during the analysis: 1. Memory leak caused by temp_buf not being freed; 2. Memory leak caused when handling argv; 3. Conditional jump that depended on unitialized values. Issue #1 -------- DESCRIPTION: when mysqlbinlog is reading from a remote location the event temp_buf references the incoming stream (in NET object), which is not freed by mysqlbinlog explicitly. On the other hand, when it is reading local binary log, it points to a temporary buffer that needs to be explicitly freed. For both cases, the temp_buf was not freed by mysqlbinlog, instead was set to 0. This clearly disregards the free required in the second case, thence creating a memory leak. FIX: we make temp_buf to be conditionally freed depending on the value of remote_opt. Found out that similar fix is already in most recent codebases. Issue #2 -------- DESCRIPTION: load_defaults is called by parse_args, and it reads default options from configuration files and put them BEFORE the arguments that are already in argc and argv. This is done resorting to MEM_ROOT. However, parse_args calls handle_options immediately after which changes argv. Later when freeing the defaults, pointers to MEM_ROOT won't match, causing the memory not to be freed: void free_defaults(char **argv) { MEM_ROOT ptr memcpy_fixed((char*) &ptr,(char *) argv - sizeof(ptr), sizeof(ptr)); free_root(&ptr,MYF(0)); } FIX: we remove load_defaults from parse_args and call it before. Then we save argv with defaults in defaults_argv BEFORE calling parse_args (which inside can then call handle_options at will). Actually, found out that this is in fact kind of a backport for BUG#38468 into 5.1, so I merged in the test case as well and added error check for load_defaults call. Fix based on: revid:zhenxing.he@sun.com-20091002081840-uv26f0flw4uvo33y Issue #3 -------- DESCRIPTION: the structure st_print_event_info constructor would not initialize the sql_mode member, although it did for sql_mode_inited (set to false). This would later raise the warning in valgrind when printing the sql_mode in the event header, as this print out is protected by a check against sql_mode_inited and sql_mode variables. Given that sql_mode was not initialized valgrind would output the warning. FIX: we add initialization of sql_mode to the st_print_event_info constructor.
This commit is contained in:
parent
780566d4c5
commit
fbf595d0fb
@ -832,7 +832,11 @@ Exit_status process_event(PRINT_EVENT_INFO *print_event_info, Log_event *ev,
|
||||
print_event_info->common_header_len=
|
||||
glob_description_event->common_header_len;
|
||||
ev->print(result_file, print_event_info);
|
||||
ev->temp_buf= 0; // as the event ref is zeroed
|
||||
if (!remote_opt)
|
||||
ev->free_temp_buf(); // free memory allocated in dump_local_log_entries
|
||||
else
|
||||
// disassociate but not free dump_remote_log_entries time memory
|
||||
ev->temp_buf= 0;
|
||||
/*
|
||||
We don't want this event to be deleted now, so let's hide it (I
|
||||
(Guilhem) should later see if this triggers a non-serious Valgrind
|
||||
@ -1362,7 +1366,6 @@ static int parse_args(int *argc, char*** argv)
|
||||
int ho_error;
|
||||
|
||||
result_file = stdout;
|
||||
load_defaults("my",load_default_groups,argc,argv);
|
||||
if ((ho_error=handle_options(argc, argv, my_long_options, get_one_option)))
|
||||
exit(ho_error);
|
||||
if (debug_info_flag)
|
||||
@ -2019,8 +2022,10 @@ int main(int argc, char** argv)
|
||||
|
||||
my_init_time(); // for time functions
|
||||
|
||||
parse_args(&argc, (char***)&argv);
|
||||
if (load_defaults("my", load_default_groups, &argc, &argv))
|
||||
exit(1);
|
||||
defaults_argv= argv;
|
||||
parse_args(&argc, (char***)&argv);
|
||||
|
||||
if (!argc)
|
||||
{
|
||||
|
@ -443,3 +443,27 @@ FLUSH LOGS;
|
||||
--echo End of 5.0 tests
|
||||
|
||||
--echo End of 5.1 tests
|
||||
|
||||
#
|
||||
# BUG#38468 Memory leak detected when using mysqlbinlog utility;
|
||||
#
|
||||
disable_query_log;
|
||||
RESET MASTER;
|
||||
CREATE TABLE t1 SELECT 1;
|
||||
FLUSH LOGS;
|
||||
DROP TABLE t1;
|
||||
enable_query_log;
|
||||
|
||||
# Write an empty file for comparison
|
||||
write_file $MYSQLTEST_VARDIR/tmp/mysqlbinlog.warn.empty;
|
||||
EOF
|
||||
|
||||
# Before fix of BUG#38468, this would generate some warnings
|
||||
--exec $MYSQL_BINLOG $MYSQLD_DATADIR/master-bin.000001 >/dev/null 2> $MYSQLTEST_VARDIR/tmp/mysqlbinlog.warn
|
||||
|
||||
# Make sure the command above does not generate any error or warnings
|
||||
diff_files $MYSQLTEST_VARDIR/tmp/mysqlbinlog.warn $MYSQLTEST_VARDIR/tmp/mysqlbinlog.warn.empty;
|
||||
|
||||
# Cleanup for this part of test
|
||||
remove_file $MYSQLTEST_VARDIR/tmp/mysqlbinlog.warn.empty;
|
||||
remove_file $MYSQLTEST_VARDIR/tmp/mysqlbinlog.warn;
|
||||
|
@ -9478,7 +9478,7 @@ Incident_log_event::write_data_body(IO_CACHE *file)
|
||||
they will always be printed for the first event.
|
||||
*/
|
||||
st_print_event_info::st_print_event_info()
|
||||
:flags2_inited(0), sql_mode_inited(0),
|
||||
:flags2_inited(0), sql_mode_inited(0), sql_mode(0),
|
||||
auto_increment_increment(0),auto_increment_offset(0), charset_inited(0),
|
||||
lc_time_names_number(~0),
|
||||
charset_database_number(ILLEGAL_CHARSET_INFO_NUMBER),
|
||||
|
Loading…
x
Reference in New Issue
Block a user