Problem:
========
The mysqlbinlog tool is leaking memory, causing failures in various tests when
compiling and testing with AddressSanitizer or LeakSanitizer like this:
cmake -DCMAKE_BUILD_TYPE=Debug -DWITH_ASAN:BOOL=ON /path/to/source
make -j$(nproc)
cd mysql-test
ASAN_OPTIONS=abort_on_error=1 ./mtr --parallel=auto
Analysis:
=========
Two types of leaks were observed during above execution.
1) Leak in Log_event::read_log_event(char const*, unsigned int, char const**,
Format_description_log_event const*, char)
File: sql/log_event.cc:2150
For all row based replication events the memory which is allocated during
read_log_event is not freed after the event is processed. The event specific
memory has to be retained only when flashback option is enabled with
mysqlbinlog tool. In this case all the events are retained till the end
statement is received and they are processed in reverse order and they are
destroyed. But in the existing code all events are retained irrespective of
flashback mode. Hence the memory leaks are observed.
2) read_remote_annotate_event(unsigned char*, unsigned long, char const**)
File: client/mysqlbinlog.cc:194
In general the Annotate event is not printed immediately because all
subsequent rbr-events can be filtered away. Instead it will be printed
together with the first not filtered away Table map or the last rbr will be
processed. While reading remote annotate events memory is allocated for event
buffer and event's temp_buf is made to point to the allocated buffer as shown
below. The TRUE flag is used for doing proper cleanup using free_temp_buf().
i.e at the time of deletion of annotate event its destructor takes care of
clearing the temp_buf.
/*
Ensure the event->temp_buf is pointing to the allocated buffer.
(TRUE = free temp_buf on the event deletion)
*/
event->register_temp_buf((char*)event_buf, TRUE);
But existing code does the following when it receives a remote annotate_event.
if (remote_opt)
ev->temp_buf= 0;
That is code immediately sets temp_buf=0, because of which free_temp_buf()
call will return empty handed as it has lost the reference to the allocated
temporary buffer. This results in memory leak
Fix:
====
1) If not in flashback mode, destroy the memory for events once they are
processed.
2) Remove the ev->temp_buf=0 code for remote option. Let the proper cleanup to
be done as part of free_temp_buf().
Problem:
========
The mysqlbinlog tool is leaking memory, causing failures in various tests when
compiling and testing with AddressSanitizer or LeakSanitizer like this:
cmake -DCMAKE_BUILD_TYPE=Debug -DWITH_ASAN:BOOL=ON /path/to/source
make -j$(nproc)
cd mysql-test
ASAN_OPTIONS=abort_on_error=1 ./mtr --parallel=auto rpl.rpl_row_mysqlbinlog
CURRENT_TEST: rpl.rpl_row_mysqlbinlog
Direct leak of 112 byte(s) in 1 object(s) allocated from:
#0 0x4eff87 in __interceptor_malloc (/dev/shm/5.5/client/mysqlbinlog+0x4eff87)
#1 0x60eaab in my_malloc /mariadb/5.5/mysys/my_malloc.c:41:10
#2 0x5300dd in Log_event::read_log_event(char const*, unsigned int, char const**,
Format_description_log_event const*, char) /mariadb/5.5/sql/log_event.cc:1568:
#3 0x564a9c in dump_remote_log_entries(st_print_event_info*, char const*)
/mariadb/5.5/client/mysqlbinlog.cc:1978:17
Analysis:
========
'mysqlbinlog' tool is being used to read binary log events from a remote server.
While reading binary log, if a fake rotate event is found following actions are
taken.
If 'to-last-log' option is specified, then fake rotate event is processed.
In the absence of 'to-last-log' skip the fake rotate event.
In this skipped case the fake rotate event object is not getting cleaned up
resulting in memory leak.
Fix:
===
Cleanup the fake rotate event.
This issues is already fixed in MariaDB 10.0.23 and higher versions as part of
commit c3018b0ff4
There were two newly enabled warnings:
1. cast for a function pointers. Affected sql_analyse.h, mi_write.c
and ma_write.cc, mf_iocache-t.cc, mysqlbinlog.cc, encryption.cc, etc
2. memcpy/memset of nontrivial structures. Fixed as:
* the warning disabled for InnoDB
* TABLE, TABLE_SHARE, and TABLE_LIST got a new method reset() which
does the bzero(), which is safe for these classes, but any other
bzero() will still cause a warning
* Table_scope_and_contents_source_st uses `TABLE_LIST *` (trivial)
instead of `SQL_I_List<TABLE_LIST>` (not trivial) so it's safe to
bzero now.
* added casts in debug_sync.cc and sql_select.cc (for JOIN)
* move assignment method for MDL_request instead of memcpy()
* PARTIAL_INDEX_INTERSECT_INFO::init() instead of bzero()
* remove constructor from READ_RECORD() to make it trivial
* replace some memcpy() with c++ copy assignments
Make mysqltest to use --ps-protocol more
use prepared statements for everything that server supports
with the exception of CALL (for now).
Fix discovered test failures and bugs.
tests:
* PROCESSLIST shows Execute state, not Query
* SHOW STATUS increments status variables more than in text protocol
* multi-statements should be avoided (see tests with a wrong delimiter)
* performance_schema events have different names in --ps-protocol
* --enable_prepare_warnings
mysqltest.cc:
* make sure run_query_stmt() doesn't crash if there's
no active connection (in wait_until_connected_again.inc)
* prepare all statements that server supports
protocol.h
* Protocol_discard::send_result_set_metadata() should not send
anything to the client.
sql_acl.cc:
* extract the functionality of getting the user for SHOW GRANTS
from check_show_access(), so that mysql_test_show_grants() could
generate the correct column names in the prepare step
sql_class.cc:
* result->prepare() can fail, don't ignore its return value
* use correct number of decimals for EXPLAIN columns
sql_parse.cc:
* discard profiling for SHOW PROFILE. In text protocol it's done in
prepare_schema_table(), but in --ps it is called on prepare only,
so nothing was discarding profiling during execute.
* move the permission checking code for SHOW CREATE VIEW to
mysqld_show_create_get_fields(), so that it would be called during
prepare step too.
* only set sel_result when it was created here and needs to be
destroyed in the same block. Avoid destroying lex->result.
* use the correct number of tables in check_show_access(). Saying
"as many as possible" doesn't work when first_not_own_table isn't
set yet.
sql_prepare.cc:
* use correct user name for SHOW GRANTS columns
* don't ignore verbose flag for SHOW SLAVE STATUS
* support preparing REVOKE ALL and ROLLBACK TO SAVEPOINT
* don't ignore errors from thd->prepare_explain_fields()
* use select_send result for sending ANALYZE and EXPLAIN, but don't
overwrite lex->result, because it might be needed to issue execute-time
errors (select_dumpvar - too many rows)
sql_show.cc:
* check grants for SHOW CREATE VIEW here, not in mysql_execute_command
sql_view.cc:
* use the correct function to check privileges. Old code was doing
check_access() for thd->security_ctx, which is invoker's sctx,
not definer's sctx. Hide various view related errors from the invoker.
sql_yacc.yy:
* initialize lex->select_lex for LOAD, otherwise it'll contain garbage
data that happen to fail tests with views in --ps (but not otherwise).
Disable LOAD DATA LOCAL INFILE suport by default and
auto-enable it for the duration of one query, if the query
string starts with the word "load". In all other cases the application
should enable LOAD DATA LOCAL INFILE support explicitly.
The problem was originally stated in
http://bugs.mysql.com/bug.php?id=82212
The size of an base64-encoded Rows_log_event exceeds its
vanilla byte representation in 4/3 times.
When a binlogged event size is about 1GB mysqlbinlog generates
a BINLOG query that can't be send out due to its size.
It is fixed with fragmenting the BINLOG argument C-string into
(approximate) halves when the base64 encoded event is over 1GB size.
The mysqlbinlog in such case puts out
SET @binlog_fragment_0='base64-encoded-fragment_0';
SET @binlog_fragment_1='base64-encoded-fragment_1';
BINLOG @binlog_fragment_0, @binlog_fragment_1;
to represent a big BINLOG.
For prompt memory release BINLOG handler is made to reset the BINLOG argument
user variables in the middle of processing, as if @binlog_fragment_{0,1} = NULL
is assigned.
Notice the 2 fragments are enough, though the client and server still may
need to tweak their @@max_allowed_packet to satisfy to the fragment
size (which they would have to do anyway with greater number of
fragments, should that be desired).
On the lower level the following changes are made:
Log_event::print_base64()
remains to call encoder and store the encoded data into a cache but
now *without* doing any formatting. The latter is left for time
when the cache is copied to an output file (e.g mysqlbinlog output).
No formatting behavior is also reflected by the change in the meaning
of the last argument which specifies whether to cache the encoded data.
Rows_log_event::print_helper()
is made to invoke a specialized fragmented cache-to-file copying function
which is
copy_cache_to_file_wrapped()
that takes care of fragmenting also optionally wraps encoded
strings (fragments) into SQL stanzas.
my_b_copy_to_file()
is refactored to into my_b_copy_all_to_file(). The former function
is generalized
to accepts more a limit argument to constraint the copying and does
not reinitialize anymore the cache into reading mode.
The limit does not do any effect on the fully read cache.
Close connection handler on connection failure. This fixes 14 failing tests in
main suite under clang+ASAN build.
ASAN report for main.connect looks like this:
=================================================================
==25495==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 146280 byte(s) in 115 object(s) allocated from:
#0 0x4fba47 in calloc /fun/cpp_projects/llvm_toolchain/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:138
#1 0x5a7a02 in mysql_init /work/mariadb/libmariadb/libmariadb/mariadb_lib.c:977:26
#2 0x570a7a in do_connect(st_command*) /work/mariadb/client/mysqltest.cc:6096:26
#3 0x584c39 in main /work/mariadb/client/mysqltest.cc:9321:9
#4 0x7fd15514db96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
Indirect leak of 7065600 byte(s) in 115 object(s) allocated from:
#0 0x4fb80f in __interceptor_malloc /fun/cpp_projects/llvm_toolchain/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:129
#1 0x637a83 in my_context_init /work/mariadb/libmariadb/libmariadb/ma_context.c:367:23
#2 0x59fd16 in mysql_optionsv /work/mariadb/libmariadb/libmariadb/mariadb_lib.c:2738:9
#3 0x5bc1d4 in mysql_options /work/mariadb/libmariadb/libmariadb/mariadb_lib.c:3242:10
#4 0x570b94 in do_connect(st_command*) /work/mariadb/client/mysqltest.cc:6103:7
#5 0x584c39 in main /work/mariadb/client/mysqltest.cc:9321:9
#6 0x7fd15514db96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
Indirect leak of 940240 byte(s) in 115 object(s) allocated from:
#0 0x4fb80f in __interceptor_malloc /fun/cpp_projects/llvm_toolchain/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:129
#1 0x64386e in ma_init_dynamic_array /work/mariadb/libmariadb/libmariadb/ma_array.c:49:31
#2 0x649ead in _hash_init /work/mariadb/libmariadb/libmariadb/ma_hash.c:52:7
#3 0x5a3080 in mysql_optionsv /work/mariadb/libmariadb/libmariadb/mariadb_lib.c:2938:13
#4 0x5bc20c in mysql_options4 /work/mariadb/libmariadb/libmariadb/mariadb_lib.c:3248:10
#5 0x56f63b in connect_n_handle_errors(st_command*, st_mysql*, char const*, char const*, char const*, char const*, int, char const*) /work/mariadb/client/mysqltest.cc:5874:3
#6 0x57146b in do_connect(st_command*) /work/mariadb/client/mysqltest.cc:6193:7
#7 0x584c39 in main /work/mariadb/client/mysqltest.cc:9321:9
#8 0x7fd15514db96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
...
Closes#809
Also, apply the MDEV-17957 changes to encrypted page checksums,
and remove error message output from the checksum function,
because these messages would be useless noise when mariabackup
is retrying reads of corrupted-looking pages, and not that
useful during normal server operation either.
The error messages in fil_space_verify_crypt_checksum()
should be refactored separately.
