maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

Hi Oleg,

> commit 4cad1e16a85114815075e5706d6a8faf5e8fc2e5
> Author: Oleg Smirnov <olernov@xxxxxxxxx>
> 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()

I think the patch is ok to push except for this part:

> diff --git a/sql/sql_select.cc b/sql/sql_select.cc
> index a91b4571b21..4b8956132e8 100644
> --- a/sql/sql_select.cc
> +++ b/sql/sql_select.cc
> @@ -14029,11 +14030,11 @@ void JOIN::cleanup(bool full)
>      delete pushdown_query;
>      pushdown_query= 0;
> 
> -    if (!join_tab)
> +    List_iterator<TABLE_LIST> li(*join_list);
> +    TABLE_LIST *table_ref;
> +    while ((table_ref= li++))
>      {
> -      List_iterator<TABLE_LIST> li(*join_list);
> -      TABLE_LIST *table_ref;
> -      while ((table_ref= li++))
> +      if (!join_tab)
>        {
>          if (table_ref->table &&
>              table_ref->jtbm_subselect &&
> @@ -14043,6 +14044,14 @@ void JOIN::cleanup(bool full)
>            table_ref->table= NULL;
>          }
>        }
> +      if (table_ref->pushdown_derived)
> +      {
> +        delete table_ref->pushdown_derived;
> +        table_ref->pushdown_derived= NULL;
> +      }
> +      delete table_ref->dt_handler;
> +      table_ref->dt_handler= NULL;
> +
>      }
>    }
>    /* Restore ref array to original state */

What do you think of moving the cleanup to JOIN_TAB::cleanup()?  I did
the following and it seems to work:

diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index a91b4571b21..11cf3e13365 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -13529,6 +13530,16 @@ void JOIN_TAB::cleanup()
   select= 0;
   delete quick;
   quick= 0;
+  if (table && table->pos_in_table_list)
+  {
+    TABLE_LIST *tl= table->pos_in_table_list;
+
+    delete tl->pushdown_derived;
+    tl->pushdown_derived= NULL;
+
+    delete tl->dt_handler;
+    tl->dt_handler= NULL;
+  }
   if (rowid_filter)
   {
     delete rowid_filter;

BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net