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

17 files changed, 304 insertions, 2 deletions

diff --git a/include/my_base.h b/include/my_base.h
index 80ba4bcd004..ffd68046029 100644
--- a/include/my_base.h
+++ b/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)
diff --git a/mysql-test/suite/innodb/r/innodb_bug13635833.result b/mysql-test/suite/innodb/r/innodb_bug13635833.result
new file mode 100644
index 00000000000..e9107d5c125
--- /dev/null
+++ b/mysql-test/suite/innodb/r/innodb_bug13635833.result
@@ -0,0 +1,45 @@
+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';
diff --git a/mysql-test/suite/innodb/t/innodb_bug13635833.test b/mysql-test/suite/innodb/t/innodb_bug13635833.test
new file mode 100644
index 00000000000..b66b99b7681
--- /dev/null
+++ b/mysql-test/suite/innodb/t/innodb_bug13635833.test
@@ -0,0 +1,64 @@
+--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';
diff --git a/mysql-test/suite/innodb_plugin/r/innodb_bug13635833.result b/mysql-test/suite/innodb_plugin/r/innodb_bug13635833.result
new file mode 100644
index 00000000000..e9107d5c125
--- /dev/null
+++ b/mysql-test/suite/innodb_plugin/r/innodb_bug13635833.result
@@ -0,0 +1,45 @@
+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';
diff --git a/mysql-test/suite/innodb_plugin/t/innodb_bug13635833.test b/mysql-test/suite/innodb_plugin/t/innodb_bug13635833.test
new file mode 100644
index 00000000000..90df623e8a0
--- /dev/null
+++ b/mysql-test/suite/innodb_plugin/t/innodb_bug13635833.test
@@ -0,0 +1,64 @@
+--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';
diff --git a/mysys/my_handler_errors.h b/mysys/my_handler_errors.h
index 44c79466e84..1710ac81395 100644
--- a/mysys/my_handler_errors.h
+++ b/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 */
 };
 
diff --git a/sql/handler.cc b/sql/handler.cc
index 895e7cc4df0..d2eaab19ad8 100644
--- a/sql/handler.cc
+++ b/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.
diff --git a/storage/innobase/dict/dict0dict.c b/storage/innobase/dict/dict0dict.c
index beea0a2f411..b2baa81b73f 100644
--- a/storage/innobase/dict/dict0dict.c
+++ b/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);
 }
 
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
index 20ab9c7a101..c59c02abe1b 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/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);
diff --git a/storage/innobase/include/db0err.h b/storage/innobase/include/db0err.h
index bab6fa46370..1514b19639c 100644
--- a/storage/innobase/include/db0err.h
+++ b/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
diff --git a/storage/innobase/row/row0ins.c b/storage/innobase/row/row0ins.c
index bb5c95a572b..785a0426195 100644
--- a/storage/innobase/row/row0ins.c
+++ b/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);
diff --git a/storage/innobase/row/row0mysql.c b/storage/innobase/row/row0mysql.c
index 6148b01af9d..e8b3c485b0e 100644
--- a/storage/innobase/row/row0mysql.c
+++ b/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
diff --git a/storage/innodb_plugin/dict/dict0dict.c b/storage/innodb_plugin/dict/dict0dict.c
index 2d2262acf87..b70d5929f53 100644
--- a/storage/innodb_plugin/dict/dict0dict.c
+++ b/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);
 }
 
diff --git a/storage/innodb_plugin/handler/ha_innodb.cc b/storage/innodb_plugin/handler/ha_innodb.cc
index 09094c0146e..47d309f2360 100644
--- a/storage/innodb_plugin/handler/ha_innodb.cc
+++ b/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);
 
diff --git a/storage/innodb_plugin/include/db0err.h b/storage/innodb_plugin/include/db0err.h
index c7fa6d2a444..e4a3844cd22 100644
--- a/storage/innodb_plugin/include/db0err.h
+++ b/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,
diff --git a/storage/innodb_plugin/row/row0ins.c b/storage/innodb_plugin/row/row0ins.c
index 9a603d5690f..2d4afa6c5ed 100644
--- a/storage/innodb_plugin/row/row0ins.c
+++ b/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);
diff --git a/storage/innodb_plugin/row/row0mysql.c b/storage/innodb_plugin/row/row0mysql.c
index 36aad4f8ca2..cd46244eb34 100644
--- a/storage/innodb_plugin/row/row0mysql.c
+++ b/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. */