[MDEV-29624] Memory leak on pushdown of a merged derived table Created: 2022-09-24  Updated: 2022-11-07  Resolved: 2022-11-01

Status: Closed
Project: MariaDB Server
Component/s: Optimizer
Affects Version/s: 10.11.0
Fix Version/s: 10.11.1, 10.4.28, 10.5.19, 10.6.12, 10.7.8, 10.8.7, 10.9.5, 10.10.3

Type: Bug Priority: Major
Reporter: Oleg Smirnov Assignee: Oleg Smirnov
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Relates
relates to MDEV-29505 LeakSanitizer errors in TABLE_LIST::f... Open
relates to MDEV-29655 ASAN heap-use-after-free in Pushdown_... Closed
relates to MDEV-25080 Allow pushdown of queries involving U... Closed

 Description   

Test case below to reproduce the bug using FederatedX engine.

On the first server (remote, runs on 127.0.0.1 port 3310):

CREATE DATABASE mdev;
USE mdev;
CREATE TABLE `t1` ( `a` int(11)) ENGINE=InnoDB;

On the second server (local):

CREATE TABLE `t1` ( `a` int(11)) ENGINE=FEDERATED CONNECTION='mysql://root@127.0.0.1:3310/mdev/t1'
 
CREATE TABLE `t2` (`a` int(11)) ENGINE=InnoDB;
 
SELECT * FROM t2, (SELECT * FROM t1 WHERE a > 3) dt WHERE t2.a=dt.a;



 Comments   
Comment by Oleg Smirnov [ 2022-09-25 ]

The leak most probably happens due to duplicate allocation of derived->pushdown_derived. This is a piece of code from mysql_derived_optimize() where the allocation is being made:

  if (derived->is_materialized_derived() && derived->dt_handler)
  {
    /* Create an object for execution of the query specifying the table */
    if (!(derived->pushdown_derived=
            new (thd->mem_root) Pushdown_derived(derived, derived->dt_handler)))
    {
      delete derived->dt_handler;
      derived->dt_handler= NULL;
      DBUG_RETURN(TRUE);
    }
  }

Call stack of the first call of mysql_derived_optimize():

>	server.dll!mysql_derived_optimize(THD * thd, LEX * lex, TABLE_LIST * derived) Line 998	C++
 	server.dll!mysql_handle_single_derived(LEX * lex, TABLE_LIST * derived, unsigned int phases) Line 200	C++
 	server.dll!JOIN::optimize_inner() Line 2348	C++
 	server.dll!JOIN::optimize() Line 1869	C++
 	server.dll!mysql_select(THD * thd, TABLE_LIST * tables, List<Item> & fields, Item * conds, unsigned int og_num, st_order * order, st_order * group, Item * having, st_order * proc_param, unsigned __int64 select_options, select_result * result, st_select_lex_unit * unit, st_select_lex * select_lex) Line 5064	C++
 	server.dll!handle_select(THD * thd, LEX * lex, select_result * result, unsigned long setup_tables_done_option) Line 583	C++
 	server.dll!execute_sqlcom_select(THD * thd, TABLE_LIST * all_tables) Line 6261	C++
 	server.dll!mysql_execute_command(THD * thd, bool is_called_from_prepared_stmt) Line 3945	C++
 	server.dll!mysql_parse(THD * thd, char * rawbuf, unsigned int length, Parser_state * parser_state) Line 8037	C++
 	server.dll!dispatch_command(enum_server_command command, THD * thd, char * packet, unsigned int packet_length, bool blocking) Line 1896	C++
 	server.dll!do_command(THD * thd, bool blocking) Line 1407	C++
 	server.dll!threadpool_process_request(THD * thd) Line 402	C++
 	server.dll!tp_callback(TP_connection * c) Line 203	C++
 	server.dll!tp_callback(_TP_CALLBACK_INSTANCE * instance, void * context) Line 279	C++
 	server.dll!io_completion_callback(_TP_CALLBACK_INSTANCE * instance, void * context, void * overlapped, unsigned long io_result, unsigned __int64 nbytes, _TP_IO * io) Line 299	C++

And the second one:

>	server.dll!mysql_derived_optimize(THD * thd, LEX * lex, TABLE_LIST * derived) Line 998	C++
 	server.dll!mysql_handle_single_derived(LEX * lex, TABLE_LIST * derived, unsigned int phases) Line 200	C++
 	server.dll!TABLE_LIST::handle_derived(LEX * lex, unsigned int phases) Line 9462	C++
 	server.dll!LEX::handle_list_of_derived(TABLE_LIST * table_list, unsigned int phases) Line 4501	C++
 	server.dll!st_select_lex::handle_derived(LEX * lex, unsigned int phases) Line 4989	C++
 	server.dll!JOIN::optimize_stage2() Line 2583	C++
 	server.dll!JOIN::optimize_inner() Line 2556	C++
 	server.dll!JOIN::optimize() Line 1869	C++
 	server.dll!mysql_select(THD * thd, TABLE_LIST * tables, List<Item> & fields, Item * conds, unsigned int og_num, st_order * order, st_order * group, Item * having, st_order * proc_param, unsigned __int64 select_options, select_result * result, st_select_lex_unit * unit, st_select_lex * select_lex) Line 5064	C++
 	server.dll!handle_select(THD * thd, LEX * lex, select_result * result, unsigned long setup_tables_done_option) Line 583	C++
 	server.dll!execute_sqlcom_select(THD * thd, TABLE_LIST * all_tables) Line 6261	C++
 	server.dll!mysql_execute_command(THD * thd, bool is_called_from_prepared_stmt) Line 3945	C++
 	server.dll!mysql_parse(THD * thd, char * rawbuf, unsigned int length, Parser_state * parser_state) Line 8037	C++
 	server.dll!dispatch_command(enum_server_command command, THD * thd, char * packet, unsigned int packet_length, bool blocking) Line 1896	C++
 	server.dll!do_command(THD * thd, bool blocking) Line 1407	C++
 	server.dll!threadpool_process_request(THD * thd) Line 402	C++
 	server.dll!tp_callback(TP_connection * c) Line 203	C++
 	server.dll!tp_callback(_TP_CALLBACK_INSTANCE * instance, void * context) Line 279	C++
 	server.dll!io_completion_callback(_TP_CALLBACK_INSTANCE * instance, void * context, void * overlapped, unsigned long io_result, unsigned __int64 nbytes, _TP_IO * io) Line 299	C++

Comment by Oleg Smirnov [ 2022-10-02 ]

serg, can you please review the branch bb-10.11-MDEV-29624?

Comment by Sergei Golubchik [ 2022-10-16 ]

It'd be better if psergei reviews it.

Still, 1) there was no test case there and 2) is it 10.11 bug or should it be fixed in an earlier version?

Comment by Oleg Smirnov [ 2022-10-19 ]

1) The test case is already present in federated.federatedx_create_handlers.test, lines 95-97:

SELECT *
FROM federated.t3, (SELECT * FROM federated.t1 WHERE id > 3) t
WHERE federated.t3.name=t.name;

2) This bug is revealed only after another bug-fix which was committed into preview-10.11-mdev-25080-union-pushdown branch:

commit 2f37c2dfa1a2050e122e026ec0801bba9ba98cfd (origin/preview-10.11-mdev-25080-union-pushdown)
Author: Oleg Smirnov <olernov@gmail.com>
Date:   Thu Sep 15 18:42:28 2022 +0700
 
    MDEV-25080 Fix pushdown of statements including derived tables
    
    Derived tables sometimes prevent a SELECT or UNIT statement from being
    pushed down as a whole. This commit fixes this by adding a recoursive
    decent down the derived tables and checking tables more accurately

I don't think we need to backport that memory leak fix unless we are planning to backport this one.
This last bug has been discovered by tntnatbry during MDEV-25080 testing. I didn't file a separate Jira task, just pushed the fix to preview-10.11-mdev-25080-union-pushdown.

Comment by Oleg Smirnov [ 2022-10-19 ]

Most probably I am wrong with statement 2) from my previous comment. We just don't have test cases which reveal the bug. This commit backported to 10.4 fixes both this issue and MDEV-29655:

commit ce4899f69fd4676a5df1cc7713d8a55e0aa34d9c (HEAD -> bb-10.4-MDEV-29624-MDEV-29655, origin/bb-10.4-MDEV-29624-MDEV-29655)
Author: Oleg Smirnov <olernov@gmail.com>
Date:   Wed Oct 19 13:26:19 2022 +0400
 
    MDEV-29624 MDEV-29655 Fix ASAN errors on pushdown of derived table
    
    Deallocation of TABLE_LIST::dt_handler and TABLE_LIST::pushdown_derived
    was performed in multiple places if code. This not only made the code
    more difficult to maintain but also led to memory leaks and
    ASAN heap-use-after-free errors.
    This commit puts deallocation of TABLE_LIST::dt_handler and
    TABLE_LIST::pushdown_derived to the single point - JOIN::cleanup()

Comment by Sergei Petrunia [ 2022-10-23 ]

Review input: https://lists.launchpad.net/maria-developers/msg13251.html

Comment by Oleg Smirnov [ 2022-11-01 ]

Pushed to 10.4.

Generated at Thu Feb 08 10:10:03 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.