Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-20938

Double free of dict_foreign_t during instant ALTER TABLE

Details

    Description

      ALTER TABLE of this kind crashes 10.4.7:

      ALTER TABLE myTable
      DROP FOREIGN KEY fk_1_field001,
      DROP FOREIGN KEY fk_2_field001,
      CHANGE id_field id_field_old INT(10) UNSIGNED NOT NULL,
      ADD id_field INT(10) UNSIGNED after id_field_old,
      CHANGE id_option id_option_old INT(10) UNSIGNED NOT NULL,
      ADD id_option INT(10) UNSIGNED after id_option_old;
      

      This is a part of the stack trace:

      bits/stl_tree.h:1203(std::_Rb_tree<dict_foreign_t*, dict_foreign_t*, std::_Identity<dict_foreign_t*>, dict_foreign_compare, ut_allocator<dict_foreign_t*, true> >::equal_
      range(dict_foreign_t* const&))[0x55db4ca14a69]
      bits/stl_tree.h:1756(std::_Rb_tree<dict_foreign_t*, dict_foreign_t*, std::_Identity<dict_foreign_t*>, dict_foreign_compare, ut_allocator<dict_foreign_t*, true> >::_M_era
      se_aux(std::_Rb_tree_const_iterator<dict_foreign_t*>, std::_Rb_tree_const_iterator<dict_foreign_t*>))[0x55db4ca0dbf4]
      bits/stl_set.h:582(std::set<dict_foreign_t*, dict_foreign_compare, ut_allocator<dict_foreign_t*, true> >::erase(dict_foreign_t* const&))[0x55db4c864729]
      handler/handler0alter.cc:9553(innobase_update_foreign_cache(ha_innobase_inplace_ctx*, THD*))[0x55db4c869670]
      handler/handler0alter.cc:11127(ha_innobase::commit_inplace_alter_table(TABLE*, Alter_inplace_info*, bool))[0x55db4c411555]
      
      

      I'll attach all the details later.

      Attachments

        Issue Links

          Activity

            I narrowed it down to this merge commit that first appeared in 10.4.7:

            commit e9c1701e11e2441435223cc7c00c467f58aaff19 (HEAD)
            Merge: 17794fb9aac f3eb82f048d
            Author: Marko Mäkelä <marko.makela@mariadb.com>
            Date:   Thu Jul 25 18:42:06 2019 +0300
             
                Merge 10.3 into 10.4
            

            The following change (applied on top of that commit) makes the crash go away. So, the condition clearly needs to be revised somehow, either in the function call or inside the function:

            diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc
            index 19c35c66885..291c029ef03 100644
            --- a/storage/innobase/handler/handler0alter.cc
            +++ b/storage/innobase/handler/handler0alter.cc
            @@ -9024,7 +9024,7 @@ innobase_rename_columns_try(
             					    cf->field->field_name.str,
             					    cf->field_name.str,
             					    ctx->need_rebuild(),
            -					    ctx->is_instant())) {
            +					    false)) {
             					return(true);
             				}
             				goto processed_field;
            @@ -9155,7 +9155,7 @@ innobase_rename_or_enlarge_column_try(
             	if (!same_name
             	    && innobase_rename_column_try(user_table, trx, table_name,
             					  col_name, f.field_name.str,
            -					  false, ctx->is_instant())) {
            +					  false, false)) {
             		DBUG_RETURN(true);
             	}
             
            

            marko Marko Mäkelä added a comment - I narrowed it down to this merge commit that first appeared in 10.4.7: commit e9c1701e11e2441435223cc7c00c467f58aaff19 (HEAD) Merge: 17794fb9aac f3eb82f048d Author: Marko Mäkelä <marko.makela@mariadb.com> Date: Thu Jul 25 18:42:06 2019 +0300   Merge 10.3 into 10.4 The following change (applied on top of that commit) makes the crash go away. So, the condition clearly needs to be revised somehow, either in the function call or inside the function: diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 19c35c66885..291c029ef03 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -9024,7 +9024,7 @@ innobase_rename_columns_try( cf->field->field_name.str, cf->field_name.str, ctx->need_rebuild(), - ctx->is_instant())) { + false)) { return(true); } goto processed_field; @@ -9155,7 +9155,7 @@ innobase_rename_or_enlarge_column_try( if (!same_name && innobase_rename_column_try(user_table, trx, table_name, col_name, f.field_name.str, - false, ctx->is_instant())) { + false, false)) { DBUG_RETURN(true); }

            It turns out that this was not caused by MDEV-19630, but instead the MDEV-19630 fix was incomplete.

            If a column was renamed and a FOREIGN KEY constraint referring to it in the same operation, together with instant ADD COLUMN, we would double-free the dict_foreign_t object corresponding to the FOREIGN KEY constraint.

            marko Marko Mäkelä added a comment - It turns out that this was not caused by MDEV-19630 , but instead the MDEV-19630 fix was incomplete. If a column was renamed and a FOREIGN KEY constraint referring to it in the same operation, together with instant ADD COLUMN , we would double-free the dict_foreign_t object corresponding to the FOREIGN KEY constraint.

            People

              thiru Thirunarayanan Balathandayuthapani
              claudio.nanni Claudio Nanni
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.