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

10 files changed, 73 insertions, 0 deletions

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. */