Marko Mäkelä added a comment - 2020-04-17 12:28 The ADD FOREIGN KEY should imply ADD INDEX(a) , which in MDEV-371 (10.4) creates an indexed virtual column. There should be an equivalent test case for 10.2 that would add a virtual column and index. I think that we must first figure out what triggers the error, and then try to remove the MDEV-371 dependency of the test case, to port the test to 10.2.

Elena Stepanova added a comment - 2020-09-25 12:11 - edited Here is a test case which ends with a seemingly identical leak and doesn't involve BLOB indexes. It does involve XA instead, I'm not sure it's any better, but at least it makes the test applicable to all versions, and all of 10.1-10.5 are affected. --source include/have_innodb.inc   CREATE TABLE t1 (pk INT , a INT , PRIMARY KEY (pk)) ENGINE=InnoDB; XA START 'xid' ; INSERT INTO t1 VALUES (1,2); --error ER_XAER_RMFAIL CREATE TABLE x AS SELECT * FROM t1;   --connect (con1,localhost,root,,test) SET foreign_key_checks= OFF , innodb_lock_wait_timeout= 1; --error ER_LOCK_WAIT_TIMEOUT ALTER TABLE t1 ADD FOREIGN KEY f (a) REFERENCES t1 (pk), LOCK=EXCLUSIVE;   # Cleanup --disconnect con1 --connection default XA END 'xid' ; XA ROLLBACK 'xid' ; DROP TABLE t1; 10.3 ASAN d9d9c30b ==2816868==ERROR: LeakSanitizer: detected memory leaks   Indirect leak of 592 byte(s) in 1 object(s) allocated from: #0 0x7f49eac37bc8 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8) #1 0x563814e6f2aa in mem_heap_create_block_func(mem_block_info_t*, unsigned long, char const*, unsigned int, unsigned long) /data/src/10.3/storage/innobase/mem/mem0mem.cc:277 #2 0x563814e6fb85 in mem_heap_add_block(mem_block_info_t*, unsigned long) /data/src/10.3/storage/innobase/mem/mem0mem.cc:378 #3 0x56381539785c in mem_heap_alloc /data/src/10.3/storage/innobase/include/mem0mem.ic:191 #4 0x563815397670 in mem_heap_zalloc /data/src/10.3/storage/innobase/include/mem0mem.ic:160 #5 0x56381539e533 in dict_mem_foreign_create() /data/src/10.3/storage/innobase/dict/dict0mem.cc:810 #6 0x563814d27e36 in innobase_get_foreign_key_info /data/src/10.3/storage/innobase/handler/handler0alter.cc:2001 #7 0x563814d4f64d in ha_innobase::prepare_inplace_alter_table(TABLE*, Alter_inplace_info*) /data/src/10.3/storage/innobase/handler/handler0alter.cc:6732 #8 0x563814739627 in handler::ha_prepare_inplace_alter_table(TABLE*, Alter_inplace_info*) /data/src/10.3/sql/handler.cc:4576 #9 0x5638141fe0c9 in mysql_inplace_alter_table /data/src/10.3/sql/sql_table.cc:7617 #10 0x56381420ec66 in mysql_alter_table(THD*, st_mysql_const_lex_string const*, st_mysql_const_lex_string const*, HA_CREATE_INFO*, TABLE_LIST*, Alter_info*, unsigned int, st_order*, bool) /data/src/10.3/sql/sql_table.cc:9946 #11 0x563814378e0e in Sql_cmd_alter_table::execute(THD*) /data/src/10.3/sql/sql_alter.cc:512 #12 0x563813fc2773 in mysql_execute_command(THD*) /data/src/10.3/sql/sql_parse.cc:6022 #13 0x563813fcecf4 in mysql_parse(THD*, char*, unsigned int, Parser_state*, bool, bool) /data/src/10.3/sql/sql_parse.cc:7810 #14 0x563813fa5d60 in dispatch_command(enum_server_command, THD*, char*, unsigned int, bool, bool) /data/src/10.3/sql/sql_parse.cc:1847 #15 0x563813fa28b4 in do_command(THD*) /data/src/10.3/sql/sql_parse.cc:1393 #16 0x56381436834e in do_handle_one_connection(CONNECT*) /data/src/10.3/sql/sql_connect.cc:1403 #17 0x563814367c08 in handle_one_connection /data/src/10.3/sql/sql_connect.cc:1308 #18 0x5638159613fc in pfs_spawn_thread /data/src/10.3/storage/perfschema/pfs.cc:1869 #19 0x7f49eaa39608 in start_thread /build/glibc-YYA7BZ/glibc-2.31/nptl/pthread_create.c:477   Indirect leak of 232 byte(s) in 1 object(s) allocated from: #0 0x7f49eac37bc8 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8) #1 0x563814e6f2aa in mem_heap_create_block_func(mem_block_info_t*, unsigned long, char const*, unsigned int, unsigned long) /data/src/10.3/storage/innobase/mem/mem0mem.cc:277 #2 0x56381539791d in mem_heap_create_func /data/src/10.3/storage/innobase/include/mem0mem.ic:375 #3 0x56381539e518 in dict_mem_foreign_create() /data/src/10.3/storage/innobase/dict/dict0mem.cc:807 #4 0x563814d27e36 in innobase_get_foreign_key_info /data/src/10.3/storage/innobase/handler/handler0alter.cc:2001 #5 0x563814d4f64d in ha_innobase::prepare_inplace_alter_table(TABLE*, Alter_inplace_info*) /data/src/10.3/storage/innobase/handler/handler0alter.cc:6732 #6 0x563814739627 in handler::ha_prepare_inplace_alter_table(TABLE*, Alter_inplace_info*) /data/src/10.3/sql/handler.cc:4576 #7 0x5638141fe0c9 in mysql_inplace_alter_table /data/src/10.3/sql/sql_table.cc:7617 #8 0x56381420ec66 in mysql_alter_table(THD*, st_mysql_const_lex_string const*, st_mysql_const_lex_string const*, HA_CREATE_INFO*, TABLE_LIST*, Alter_info*, unsigned int, st_order*, bool) /data/src/10.3/sql/sql_table.cc:9946 #9 0x563814378e0e in Sql_cmd_alter_table::execute(THD*) /data/src/10.3/sql/sql_alter.cc:512 #10 0x563813fc2773 in mysql_execute_command(THD*) /data/src/10.3/sql/sql_parse.cc:6022 #11 0x563813fcecf4 in mysql_parse(THD*, char*, unsigned int, Parser_state*, bool, bool) /data/src/10.3/sql/sql_parse.cc:7810 #12 0x563813fa5d60 in dispatch_command(enum_server_command, THD*, char*, unsigned int, bool, bool) /data/src/10.3/sql/sql_parse.cc:1847 #13 0x563813fa28b4 in do_command(THD*) /data/src/10.3/sql/sql_parse.cc:1393 #14 0x56381436834e in do_handle_one_connection(CONNECT*) /data/src/10.3/sql/sql_connect.cc:1403 #15 0x563814367c08 in handle_one_connection /data/src/10.3/sql/sql_connect.cc:1308 #16 0x5638159613fc in pfs_spawn_thread /data/src/10.3/storage/perfschema/pfs.cc:1869 #17 0x7f49eaa39608 in start_thread /build/glibc-YYA7BZ/glibc-2.31/nptl/pthread_create.c:477   SUMMARY: AddressSanitizer: 824 byte(s) leaked in 2 allocation(s). 10.3 Valgrind ade782c0 ==2816407== 592 bytes in 1 blocks are indirectly lost in loss record 3 of 4 ==2816407== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==2816407== by 0xE63CD1: mem_heap_create_block_func(mem_block_info_t*, unsigned long, char const*, unsigned int, unsigned long) (mem0mem.cc:277) ==2816407== by 0xE641C6: mem_heap_add_block(mem_block_info_t*, unsigned long) (mem0mem.cc:378) ==2816407== by 0x1129687: mem_heap_alloc(mem_block_info_t*, unsigned long) (mem0mem.ic:191) ==2816407== by 0x112955B: mem_heap_zalloc(mem_block_info_t*, unsigned long) (mem0mem.ic:160) ==2816407== by 0x112D270: dict_mem_foreign_create() (dict0mem.cc:810) ==2816407== by 0xDC37E5: innobase_get_foreign_key_info(Alter_inplace_info*, TABLE_SHARE const*, dict_table_t*, char const**, dict_index_t**, unsigned long, dict_foreign_t**, unsigned long*, trx_t const*, std::__cxx11::list<dict_s_col_t, ut_allocator<dict_s_col_t, true> >*) (handler0alter.cc:2001) ==2816407== by 0xDD57A9: ha_innobase::prepare_inplace_alter_table(TABLE*, Alter_inplace_info*) (handler0alter.cc:6732) ==2816407== by 0xB4B337: handler::ha_prepare_inplace_alter_table(TABLE*, Alter_inplace_info*) (handler.cc:4576) ==2816407== by 0x8F0932: mysql_inplace_alter_table(THD*, TABLE_LIST*, TABLE*, TABLE*, Alter_inplace_info*, enum_alter_inplace_result, MDL_request*, Alter_table_ctx*) (sql_table.cc:7617) ==2816407== by 0x8F78BE: mysql_alter_table(THD*, st_mysql_const_lex_string const*, st_mysql_const_lex_string const*, HA_CREATE_INFO*, TABLE_LIST*, Alter_info*, unsigned int, st_order*, bool) (sql_table.cc:9946) ==2816407== by 0x98C40D: Sql_cmd_alter_table::execute(THD*) (sql_alter.cc:512) ==2816407== by 0x811972: mysql_execute_command(THD*) (sql_parse.cc:6022) ==2816407== by 0x8171D7: mysql_parse(THD*, char*, unsigned int, Parser_state*, bool, bool) (sql_parse.cc:7810) ==2816407== by 0x803850: dispatch_command(enum_server_command, THD*, char*, unsigned int, bool, bool) (sql_parse.cc:1847) ==2816407== by 0x8021F0: do_command(THD*) (sql_parse.cc:1393) ==2816407== ==2816407== 824 (232 direct, 592 indirect) bytes in 1 blocks are definitely lost in loss record 4 of 4 ==2816407== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==2816407== by 0xE63CD1: mem_heap_create_block_func(mem_block_info_t*, unsigned long, char const*, unsigned int, unsigned long) (mem0mem.cc:277) ==2816407== by 0x1129791: mem_heap_create_func(unsigned long, char const*, unsigned int, unsigned long) (mem0mem.ic:375) ==2816407== by 0x112D25B: dict_mem_foreign_create() (dict0mem.cc:807) ==2816407== by 0xDC37E5: innobase_get_foreign_key_info(Alter_inplace_info*, TABLE_SHARE const*, dict_table_t*, char const**, dict_index_t**, unsigned long, dict_foreign_t**, unsigned long*, trx_t const*, std::__cxx11::list<dict_s_col_t, ut_allocator<dict_s_col_t, true> >*) (handler0alter.cc:2001) ==2816407== by 0xDD57A9: ha_innobase::prepare_inplace_alter_table(TABLE*, Alter_inplace_info*) (handler0alter.cc:6732) ==2816407== by 0xB4B337: handler::ha_prepare_inplace_alter_table(TABLE*, Alter_inplace_info*) (handler.cc:4576) ==2816407== by 0x8F0932: mysql_inplace_alter_table(THD*, TABLE_LIST*, TABLE*, TABLE*, Alter_inplace_info*, enum_alter_inplace_result, MDL_request*, Alter_table_ctx*) (sql_table.cc:7617) ==2816407== by 0x8F78BE: mysql_alter_table(THD*, st_mysql_const_lex_string const*, st_mysql_const_lex_string const*, HA_CREATE_INFO*, TABLE_LIST*, Alter_info*, unsigned int, st_order*, bool) (sql_table.cc:9946) ==2816407== by 0x98C40D: Sql_cmd_alter_table::execute(THD*) (sql_alter.cc:512) ==2816407== by 0x811972: mysql_execute_command(THD*) (sql_parse.cc:6022) ==2816407== by 0x8171D7: mysql_parse(THD*, char*, unsigned int, Parser_state*, bool, bool) (sql_parse.cc:7810) ==2816407== by 0x803850: dispatch_command(enum_server_command, THD*, char*, unsigned int, bool, bool) (sql_parse.cc:1847) ==2816407== by 0x8021F0: do_command(THD*) (sql_parse.cc:1393) ==2816407== by 0x9859D3: do_handle_one_connection(CONNECT*) (sql_connect.cc:1403) ==2816407== by 0x98572F: handle_one_connection (sql_connect.cc:1308) ==2816407==

Marko Mäkelä added a comment - 2020-09-25 13:36 elenst , thank you! The test case is relying on an MDL bug (similar to MDEV-15532 and others), and I think that it will leave the first transaction active inside InnoDB while wrongly letting the DDL operation in the second connection to proceed. I think that it is an adequate regression test. We can worry about replacing it when the MDL bug has been fixed. One possible refinement would be to move the transaction to XA PREPARE state and then kill and restart the server, to have it recover with an InnoDB table IX lock for the table, but no MDL. And even that refinement would eventually stop working when we fix the recovery so that MDL will be resurrected for the recovered transactions.

Thirunarayanan Balathandayuthapani added a comment - 2020-09-26 07:45 diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index e55eda2d331..8654f40865a 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -5949,6 +5949,12 @@ prepare_inplace_alter_table_dict( ctx->prebuilt->trx->error_info = NULL; + for (uint i = 0; i < ctx->num_to_add_fk; i++) { + if (ctx->add_fk[i]) { + dict_foreign_free(ctx->add_fk[i]); + } + } + if (!ctx->trx) { goto err_exit; } The above patch fixes the memory leak issue.

Marko Mäkelä added a comment - 2020-09-28 07:35 It seems to me that the leaked memory must have been allocated in ha_innobase::prepare_inplace_alter_table() before invoking prepare_inplace_alter_table_dict() . As far as I can tell, you are suggesting to add the loop to the function prepare_inplace_alter_table_dict() soon after the error_handled: label. There seems to be at least one code path that would bypass that code by executing goto err_exit . I think that a more appropriate place for the code would be near the end of the function, right before the delete ctx . I think that this change should be small and low-risk enough to be put to the 10.1 version.