[MDEV-20938] Double free of dict_foreign_t during instant ALTER TABLE Created: 2019-10-31  Updated: 2020-08-25  Resolved: 2019-11-01

Status: Closed
Project: MariaDB Server
Component/s: Storage Engine - InnoDB
Affects Version/s: 10.4.7, 10.4.8
Fix Version/s: 10.3.19, 10.4.9

Type: Bug Priority: Blocker
Reporter: Claudio Nanni Assignee: Thirunarayanan Balathandayuthapani
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Duplicate
PartOf
includes MDEV-19630 ALTER TABLE ... ADD COLUMN damages fo... Closed

 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.

 Comments   
Comment by Marko Mäkelä [ 2019-10-31 ]

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);
 
 	}

Comment by Marko Mäkelä [ 2019-11-01 ]

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.

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