Bug#13635833: MULTIPLE CRASHES IN FOREIGN KEY CODE WITH CONCURRENT DDL/DML

      
There are two threads.  In one thread, dml operation is going on 
involving cascaded update operation.  In another thread, alter 
table add foreign key constraint is happening.  Under these 
circumstances, it is possible for the dml thread to access a 
dict_foreign_t object that has been freed by the ddl thread.  
The debug sync test case provides the sequence of operations.  
Without fix, the test case will crash the server (because of 
newly added assert).  With fix, the alter table stmt will return 
an error message.  
      
Backporting the fix from MySQL 5.5 to 5.1

rb:961
rb:947

include/my_base.h

@@ -445,7 +445,12 @@ enum ha_base_keytype {
 #define HA_ERR_FILE_TOO_SHORT	  175	 /* File too short */
 #define HA_ERR_WRONG_CRC	  176	 /* Wrong CRC on page */
 #define HA_ERR_TOO_MANY_CONCURRENT_TRXS 177 /*Too many active concurrent transactions */
-#define HA_ERR_LAST               177    /* Copy of last error nr */
+
+/* The error codes from 178 to 180 is not used, because we need to
+maintain forward compatibility with higher versions. */
+
+#define HA_ERR_TABLE_IN_FK_CHECK  181	 /* Table being used in foreign key check */
+#define HA_ERR_LAST               181    /* Copy of last error nr */
 
 /* Number of different errors */
 #define HA_ERR_ERRORS            (HA_ERR_LAST - HA_ERR_FIRST + 1)

mysql-test/suite/innodb/r/innodb_bug13635833.result

+SET DEBUG_SYNC='reset';
+create table t1 (f1 integer, key k1 (f1)) engine=innodb;
+create table t2 (f1 int, f2 int, key(f1), key(f2)) engine=innodb;
+create table t3 (f2 int, key(f2)) engine=innodb;
+insert into t1 values (10);
+insert into t2 values (10, 20);
+insert into t3 values (20);
+alter table t2 add constraint c1 foreign key (f1)
+references t1(f1) on update cascade;
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `f1` int(11) DEFAULT NULL,
+  KEY `k1` (`f1`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1
+show create table t2;
+Table	Create Table
+t2	CREATE TABLE `t2` (
+  `f1` int(11) DEFAULT NULL,
+  `f2` int(11) DEFAULT NULL,
+  KEY `f1` (`f1`),
+  KEY `f2` (`f2`),
+  CONSTRAINT `c1` FOREIGN KEY (`f1`) REFERENCES `t1` (`f1`) ON UPDATE CASCADE
+) ENGINE=InnoDB DEFAULT CHARSET=latin1
+show create table t3;
+Table	Create Table
+t3	CREATE TABLE `t3` (
+  `f2` int(11) DEFAULT NULL,
+  KEY `f2` (`f2`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1
+SET DEBUG_SYNC='alter_table_before_rename_result_table
+   SIGNAL update_can_proceed WAIT_FOR dict_unfreeze';
+alter table t2 add constraint z1 foreign key (f2)
+references t3(f2) on update cascade;
+SET DEBUG_SYNC='innodb_row_update_for_mysql_begin
+                            WAIT_FOR update_can_proceed';
+SET DEBUG_SYNC='innodb_dml_cascade_dict_unfreeze SIGNAL dict_unfreeze
+                            WAIT_FOR  foreign_free_cache';
+update ignore t1 set f1 = 20;
+ERROR HY000: Error on rename of './test/t2' to '#sql2-temporary' (errno: 181)
+SET DEBUG_SYNC='now SIGNAL foreign_free_cache';
+drop table t2;
+drop table t1;
+drop table t3;
+SET DEBUG_SYNC='reset';

mysql-test/suite/innodb/t/innodb_bug13635833.test

+--source include/have_innodb.inc
+--source include/have_debug_sync.inc
+--source include/not_embedded.inc
+
+SET DEBUG_SYNC='reset';
+
+# Save the initial number of concurrent sessions
+--source include/count_sessions.inc
+
+create table t1 (f1 integer, key k1 (f1)) engine=innodb;
+create table t2 (f1 int, f2 int, key(f1), key(f2)) engine=innodb;
+create table t3 (f2 int, key(f2)) engine=innodb;
+
+insert into t1 values (10);
+insert into t2 values (10, 20);
+insert into t3 values (20);
+
+alter table t2 add constraint c1 foreign key (f1)
+    references t1(f1) on update cascade;
+
+show create table t1;
+show create table t2;
+show create table t3;
+
+SET DEBUG_SYNC='alter_table_before_rename_result_table
+   SIGNAL update_can_proceed WAIT_FOR dict_unfreeze';
+
+--send
+alter table t2 add constraint z1 foreign key (f2)
+    references t3(f2) on update cascade;
+
+connect (thr2,localhost,root,,);
+connection thr2;
+
+SET DEBUG_SYNC='innodb_row_update_for_mysql_begin
+                            WAIT_FOR update_can_proceed';
+SET DEBUG_SYNC='innodb_dml_cascade_dict_unfreeze SIGNAL dict_unfreeze
+                            WAIT_FOR  foreign_free_cache';
+
+--send
+update ignore t1 set f1 = 20;
+
+connection default;
+--replace_regex /'[^']*test\/#sql2-[0-9a-f-]*'/'#sql2-temporary'/
+--error ER_ERROR_ON_RENAME
+reap;
+
+SET DEBUG_SYNC='now SIGNAL foreign_free_cache';
+
+connection thr2;
+reap;
+disconnect thr2;
+--source include/wait_until_disconnected.inc
+
+connection default;
+
+drop table t2;
+drop table t1;
+drop table t3;
+
+# Wait till we reached the initial number of concurrent sessions
+--source include/wait_until_count_sessions.inc
+
+SET DEBUG_SYNC='reset';

mysql-test/suite/innodb_plugin/r/innodb_bug13635833.result

+SET DEBUG_SYNC='reset';
+create table t1 (f1 integer, key k1 (f1)) engine=innodb;
+create table t2 (f1 int, f2 int, key(f1), key(f2)) engine=innodb;
+create table t3 (f2 int, key(f2)) engine=innodb;
+insert into t1 values (10);
+insert into t2 values (10, 20);
+insert into t3 values (20);
+alter table t2 add constraint c1 foreign key (f1)
+references t1(f1) on update cascade;
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `f1` int(11) DEFAULT NULL,
+  KEY `k1` (`f1`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1
+show create table t2;
+Table	Create Table
+t2	CREATE TABLE `t2` (
+  `f1` int(11) DEFAULT NULL,
+  `f2` int(11) DEFAULT NULL,
+  KEY `f1` (`f1`),
+  KEY `f2` (`f2`),
+  CONSTRAINT `c1` FOREIGN KEY (`f1`) REFERENCES `t1` (`f1`) ON UPDATE CASCADE
+) ENGINE=InnoDB DEFAULT CHARSET=latin1
+show create table t3;
+Table	Create Table
+t3	CREATE TABLE `t3` (
+  `f2` int(11) DEFAULT NULL,
+  KEY `f2` (`f2`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1
+SET DEBUG_SYNC='alter_table_before_rename_result_table
+   SIGNAL update_can_proceed WAIT_FOR dict_unfreeze';
+alter table t2 add constraint z1 foreign key (f2)
+references t3(f2) on update cascade;
+SET DEBUG_SYNC='innodb_row_update_for_mysql_begin
+                            WAIT_FOR update_can_proceed';
+SET DEBUG_SYNC='innodb_dml_cascade_dict_unfreeze SIGNAL dict_unfreeze
+                            WAIT_FOR  foreign_free_cache';
+update ignore t1 set f1 = 20;
+ERROR HY000: Error on rename of './test/t2' to '#sql2-temporary' (errno: 181)
+SET DEBUG_SYNC='now SIGNAL foreign_free_cache';
+drop table t2;
+drop table t1;
+drop table t3;
+SET DEBUG_SYNC='reset';

mysql-test/suite/innodb_plugin/t/innodb_bug13635833.test

+--source include/have_innodb_plugin.inc
+--source include/have_debug_sync.inc
+--source include/not_embedded.inc
+
+SET DEBUG_SYNC='reset';
+
+# Save the initial number of concurrent sessions
+--source include/count_sessions.inc
+
+create table t1 (f1 integer, key k1 (f1)) engine=innodb;
+create table t2 (f1 int, f2 int, key(f1), key(f2)) engine=innodb;
+create table t3 (f2 int, key(f2)) engine=innodb;
+
+insert into t1 values (10);
+insert into t2 values (10, 20);
+insert into t3 values (20);
+
+alter table t2 add constraint c1 foreign key (f1)
+    references t1(f1) on update cascade;
+
+show create table t1;
+show create table t2;
+show create table t3;
+
+SET DEBUG_SYNC='alter_table_before_rename_result_table
+   SIGNAL update_can_proceed WAIT_FOR dict_unfreeze';
+
+--send
+alter table t2 add constraint z1 foreign key (f2)
+    references t3(f2) on update cascade;
+
+connect (thr2,localhost,root,,);
+connection thr2;
+
+SET DEBUG_SYNC='innodb_row_update_for_mysql_begin
+                            WAIT_FOR update_can_proceed';
+SET DEBUG_SYNC='innodb_dml_cascade_dict_unfreeze SIGNAL dict_unfreeze
+                            WAIT_FOR  foreign_free_cache';
+
+--send
+update ignore t1 set f1 = 20;
+
+connection default;
+--replace_regex /'[^']*test\/#sql2-[0-9a-f-]*'/'#sql2-temporary'/
+--error ER_ERROR_ON_RENAME
+reap;
+
+SET DEBUG_SYNC='now SIGNAL foreign_free_cache';
+
+connection thr2;
+reap;
+disconnect thr2;
+--source include/wait_until_disconnected.inc
+
+connection default;
+
+drop table t2;
+drop table t1;
+drop table t3;
+
+# Wait till we reached the initial number of concurrent sessions
+--source include/wait_until_count_sessions.inc
+
+SET DEBUG_SYNC='reset';

mysys/my_handler_errors.h

@@ -78,6 +78,10 @@ static const char *handler_error_messages[]=
   "Got a fatal error during initialzaction of handler",
   "File to short; Expected more data in file",
   "Read page with wrong checksum",
-  "Too many active concurrent transactions"
+  "Too many active concurrent transactions",
+  "",
+  "",
+  "",
+  "Table is being used in foreign key check" /* HA_ERR_TABLE_IN_FK_CHECK */
 };
 

sql/handler.cc

@@ -345,6 +345,7 @@ int ha_init_errors(void)
   SETMSG(HA_ERR_AUTOINC_READ_FAILED,    ER(ER_AUTOINC_READ_FAILED));
   SETMSG(HA_ERR_AUTOINC_ERANGE,         ER(ER_WARN_DATA_OUT_OF_RANGE));
   SETMSG(HA_ERR_TOO_MANY_CONCURRENT_TRXS, ER(ER_TOO_MANY_CONCURRENT_TRXS));
+  SETMSG(HA_ERR_TABLE_IN_FK_CHECK,      "Table being used in foreign key check");
 
   /* Register the error messages for use with my_error(). */
   return my_error_register(errmsgs, HA_ERR_FIRST, HA_ERR_LAST);
@@ -2820,6 +2821,7 @@ void handler::print_error(int error, myf errflag)
   case HA_ERR_TOO_MANY_CONCURRENT_TRXS:
     textno= ER_TOO_MANY_CONCURRENT_TRXS;
     break;
+  case HA_ERR_TABLE_IN_FK_CHECK:
   default:
     {
       /* The error was "unknown" to this function.

storage/innobase/dict/dict0dict.c

@@ -26,6 +26,8 @@ Created 1/8/1996 Heikki Tuuri
 #include "pars0sym.h"
 #include "que0que.h"
 #include "rem0cmp.h"
+#include "m_string.h"
+#include "my_sys.h"
 #ifndef UNIV_HOTBACKUP
 # include "m_ctype.h" /* my_isspace() */
 #endif /* !UNIV_HOTBACKUP */
@@ -1889,6 +1891,8 @@ dict_foreign_free(
 /*==============*/
 	dict_foreign_t*	foreign)	/* in, own: foreign key struct */
 {
+	ut_a(foreign->foreign_table->n_foreign_key_checks_running == 0);
+
 	mem_heap_free(foreign->heap);
 }
 

storage/innobase/handler/ha_innodb.cc

@@ -746,6 +746,10 @@ convert_error_code_to_mysql(
 
 		return(HA_ERR_RECORD_FILE_FULL);
 
+	} else if (error == (int) DB_TABLE_IN_FK_CHECK) {
+
+		return(HA_ERR_TABLE_IN_FK_CHECK);
+
 	} else if (error == (int) DB_TABLE_IS_BEING_USED) {
 
 		return(HA_ERR_WRONG_COMMAND);

storage/innobase/include/db0err.h

@@ -82,6 +82,8 @@ Created 5/24/1996 Heikki Tuuri
 #define DB_REFERENCING_NO_INDEX	52	/* the parent (referencing) table does
 					not have an index that contains the
 					foreign keys as its prefix columns */
+#define DB_TABLE_IN_FK_CHECK    53	/* table is being used in foreign
+					key check */
 
 /* The following are partial failure codes */
 #define DB_FAIL			1000

storage/innobase/row/row0ins.c

@@ -1074,6 +1074,9 @@ row_ins_foreign_check_on_constraint(
 	release the latch. */
 
 	row_mysql_unfreeze_data_dictionary(thr_get_trx(thr));
+
+	DEBUG_SYNC_C("innodb_dml_cascade_dict_unfreeze");
+
 	row_mysql_freeze_data_dictionary(thr_get_trx(thr));
 
 	mtr_start(mtr);

storage/innobase/row/row0mysql.c

@@ -31,6 +31,8 @@ Created 9/17/2000 Heikki Tuuri
 #include "btr0sea.h"
 #include "fil0fil.h"
 #include "ibuf0ibuf.h"
+#include "m_string.h"
+#include "my_sys.h"
 
 /* A dummy variable used to fool the compiler */
 ibool	row_mysql_identically_false	= FALSE;
@@ -1373,6 +1375,8 @@ row_update_for_mysql(
 		return(DB_ERROR);
 	}
 
+	DEBUG_SYNC_C("innodb_row_update_for_mysql_begin");
+
 	trx->op_info = "updating or deleting";
 
 	row_mysql_delay_if_needed();
@@ -3652,6 +3656,7 @@ row_rename_table_for_mysql(
 	ulint		n_constraints_to_drop	= 0;
 	ibool		old_is_tmp, new_is_tmp;
 	pars_info_t*	info			= NULL;
+	ulint		retry			= 0;
 
 	ut_ad(trx->mysql_thread_id == os_thread_get_curr_id());
 	ut_a(old_name != NULL);
@@ -3750,6 +3755,25 @@ row_rename_table_for_mysql(
 		}
 	}
 
+	/* Is a foreign key check running on this table? */
+	for (retry = 0; retry < 100
+	     && table->n_foreign_key_checks_running > 0; ++retry) {
+		row_mysql_unlock_data_dictionary(trx);
+		os_thread_yield();
+		row_mysql_lock_data_dictionary(trx);
+	}
+
+	if (table->n_foreign_key_checks_running > 0) {
+		ut_print_timestamp(stderr);
+		fputs(" InnoDB: Error: in ALTER TABLE ", stderr);
+		ut_print_name(stderr, trx, TRUE, old_name);
+		fprintf(stderr, "\n"
+			"InnoDB: a FOREIGN KEY check is running.\n"
+			"InnoDB: Cannot rename table.\n");
+		err = DB_TABLE_IN_FK_CHECK;
+		goto funct_exit;
+	}
+
 	/* We use the private SQL parser of Innobase to generate the query
 	graphs needed in deleting the dictionary data from system tables in
 	Innobase. Deleting a row from SYS_INDEXES table also frees the file

storage/innodb_plugin/dict/dict0dict.c

@@ -24,6 +24,8 @@ Created 1/8/1996 Heikki Tuuri
 ***********************************************************************/
 
 #include "dict0dict.h"
+#include "m_string.h"
+#include "my_sys.h"
 
 #ifdef UNIV_NONINL
 #include "dict0dict.ic"
@@ -2272,6 +2274,8 @@ dict_foreign_free(
 /*==============*/
 	dict_foreign_t*	foreign)	/*!< in, own: foreign key struct */
 {
+	ut_a(foreign->foreign_table->n_foreign_key_checks_running == 0);
+
 	mem_heap_free(foreign->heap);
 }
 

storage/innodb_plugin/handler/ha_innodb.cc

@@ -868,6 +868,9 @@ convert_error_code_to_mysql(
 	case DB_OUT_OF_FILE_SPACE:
 		return(HA_ERR_RECORD_FILE_FULL);
 
+	case DB_TABLE_IN_FK_CHECK:
+		return(HA_ERR_TABLE_IN_FK_CHECK);
+
 	case DB_TABLE_IS_BEING_USED:
 		return(HA_ERR_WRONG_COMMAND);
 

storage/innodb_plugin/include/db0err.h

@@ -97,6 +97,8 @@ enum db_err {
 	DB_FOREIGN_EXCEED_MAX_CASCADE,	/* Foreign key constraint related
 					cascading delete/update exceeds
 					maximum allowed depth */
+	DB_TABLE_IN_FK_CHECK,		/* table is being used in foreign
+					key check */
 
 	/* The following are partial failure codes */
 	DB_FAIL = 1000,

storage/innodb_plugin/row/row0ins.c

@@ -1102,6 +1102,9 @@ row_ins_foreign_check_on_constraint(
 	release the latch. */
 
 	row_mysql_unfreeze_data_dictionary(thr_get_trx(thr));
+
+	DEBUG_SYNC_C("innodb_dml_cascade_dict_unfreeze");
+
 	row_mysql_freeze_data_dictionary(thr_get_trx(thr));
 
 	mtr_start(mtr);

storage/innodb_plugin/row/row0mysql.c

@@ -51,6 +51,8 @@ Created 9/17/2000 Heikki Tuuri
 #include "btr0sea.h"
 #include "fil0fil.h"
 #include "ibuf0ibuf.h"
+#include "m_string.h"
+#include "my_sys.h"
 
 /** Provide optional 4.x backwards compatibility for 5.0 and above */
 UNIV_INTERN ibool	row_rollback_on_timeout	= FALSE;
@@ -1350,6 +1352,8 @@ row_update_for_mysql(
 		return(DB_ERROR);
 	}
 
+	DEBUG_SYNC_C("innodb_row_update_for_mysql_begin");
+
 	trx->op_info = "updating or deleting";
 
 	row_mysql_delay_if_needed();
@@ -3741,6 +3745,7 @@ row_rename_table_for_mysql(
 	ulint		n_constraints_to_drop	= 0;
 	ibool		old_is_tmp, new_is_tmp;
 	pars_info_t*	info			= NULL;
+	ulint		retry			= 0;
 
 	ut_ad(trx->mysql_thread_id == os_thread_get_curr_id());
 	ut_a(old_name != NULL);
@@ -3824,6 +3829,25 @@ row_rename_table_for_mysql(
 		}
 	}
 
+	/* Is a foreign key check running on this table? */
+	for (retry = 0; retry < 100
+	     && table->n_foreign_key_checks_running > 0; ++retry) {
+		row_mysql_unlock_data_dictionary(trx);
+		os_thread_yield();
+		row_mysql_lock_data_dictionary(trx);
+	}
+
+	if (table->n_foreign_key_checks_running > 0) {
+		ut_print_timestamp(stderr);
+		fputs(" InnoDB: Error: in ALTER TABLE ", stderr);
+		ut_print_name(stderr, trx, TRUE, old_name);
+		fprintf(stderr, "\n"
+			"InnoDB: a FOREIGN KEY check is running.\n"
+			"InnoDB: Cannot rename table.\n");
+		err = DB_TABLE_IN_FK_CHECK;
+		goto funct_exit;
+	}
+
 	/* We use the private SQL parser of Innobase to generate the query
 	graphs needed in updating the dictionary data from system tables. */