branches/zip: Fix some locking issues, mainly in fast index creation.

This should hopefully address Issue #85.

ha_innobase::add_index(): Lock the data dictionary before invoking
row_merge_rename_indexes() or row_merge_drop_indexes(), because neither
function will commit the transaction.

ha_innobase::final_drop_index(): Commit the transactions before
unlocking the data dictionary.

row_merge_drop_index(), row_merge_drop_indexes(), row_merge_rename_tables(),
row_merge_rename_indexes(): Note and assert that the data dictionary must
have been exclusively locked by the caller, because the transaction will
not be committed.

row_drop_database_for_mysql(): Commit the transaction immediately after
dropping each table.  When MySQL is holding open handles to some tables,
it can otherwise occur than the data dictionary is unlocked while the
transaction has not been committed.  This bug was introduced in r2739,
which changed the semantics of row_drop_table_for_mysql().

row_drop_database_for_mysql(): Postpone mem_free(table_name), so that
an error printout will not dereference freed memory.

handler/handler0alter.cc

@@ -798,7 +798,9 @@ error_handling:
 		const char*	old_name;
 		char*		tmp_name;
 	case DB_SUCCESS:
-		ut_ad(!dict_locked);
+		ut_a(!dict_locked);
+		row_mysql_lock_data_dictionary(trx);
+		dict_locked = TRUE;
 
 		if (!new_primary) {
 			error = row_merge_rename_indexes(trx, indexed_table);
@@ -819,9 +821,6 @@ error_handling:
 		tmp_name = innobase_create_temporary_tablename(heap, '2',
 							       old_name);
 
-		row_mysql_lock_data_dictionary(trx);
-		dict_locked = TRUE;
-
 		error = row_merge_rename_tables(innodb_table, indexed_table,
 						tmp_name, trx);
 
@@ -865,6 +864,11 @@ error:
 		if (new_primary) {
 			row_merge_drop_table(trx, indexed_table);
 		} else {
+			if (!dict_locked) {
+				row_mysql_lock_data_dictionary(trx);
+				dict_locked = TRUE;
+			}
+
 			row_merge_drop_indexes(trx, indexed_table,
 					       index, num_created);
 		}
@@ -1136,25 +1140,22 @@ ha_innobase::final_drop_index(
 		row_merge_lock_table(prebuilt->trx, prebuilt->table, LOCK_X),
 		prebuilt->table->flags, user_thd);
 
+	row_mysql_lock_data_dictionary(trx);
+
 	if (UNIV_UNLIKELY(err)) {
 
 		/* Unmark the indexes to be dropped. */
-		row_mysql_lock_data_dictionary(trx);
-
 		for (index = dict_table_get_first_index(prebuilt->table);
 		     index; index = dict_table_get_next_index(index)) {
 
 			index->to_be_dropped = FALSE;
 		}
 
-		row_mysql_unlock_data_dictionary(trx);
 		goto func_exit;
 	}
 
 	/* Drop indexes marked to be dropped */
 
-	row_mysql_lock_data_dictionary(trx);
-
 	index = dict_table_get_first_index(prebuilt->table);
 
 	while (index) {
@@ -1179,11 +1180,11 @@ ha_innobase::final_drop_index(
 #ifdef UNIV_DEBUG
 	dict_table_check_for_dup_indexes(prebuilt->table);
 #endif
-	row_mysql_unlock_data_dictionary(trx);
 
 func_exit:
 	trx_commit_for_mysql(trx);
 	trx_commit_for_mysql(prebuilt->trx);
+	row_mysql_unlock_data_dictionary(trx);
 
 	/* Flush the log to reduce probability that the .frm files and
 	the InnoDB data dictionary get out-of-sync if the user runs

include/row0merge.h

@@ -54,7 +54,9 @@ row_merge_lock_table(
 	dict_table_t*	table,		/* in: table to lock */
 	enum lock_mode	mode);		/* in: LOCK_X or LOCK_S */
 /*************************************************************************
-Drop an index from the InnoDB system tables. */
+Drop an index from the InnoDB system tables.  The data dictionary must
+have been locked exclusively by the caller, because the transaction
+will not be committed. */
 UNIV_INTERN
 void
 row_merge_drop_index(
@@ -63,8 +65,10 @@ row_merge_drop_index(
 	dict_table_t*	table,	/* in: table */
 	trx_t*		trx);	/* in: transaction handle */
 /*************************************************************************
-Drop those indexes which were created before an error occurred
-when building an index. */
+Drop those indexes which were created before an error occurred when
+building an index.  The data dictionary must have been locked
+exclusively by the caller, because the transaction will not be
+committed. */
 UNIV_INTERN
 void
 row_merge_drop_indexes(
@@ -80,7 +84,9 @@ void
 row_merge_drop_temp_indexes(void);
 /*=============================*/
 /*************************************************************************
-Rename the tables in the data dictionary. */
+Rename the tables in the data dictionary.  The data dictionary must
+have been locked exclusively by the caller, because the transaction
+will not be committed. */
 UNIV_INTERN
 ulint
 row_merge_rename_tables(
@@ -109,7 +115,9 @@ row_merge_create_temporary_table(
 	trx_t*			trx);		/* in/out: transaction
 						(sets error_state) */
 /*************************************************************************
-Rename the temporary indexes in the dictionary to permanent ones. */
+Rename the temporary indexes in the dictionary to permanent ones.  The
+data dictionary must have been locked exclusively by the caller,
+because the transaction will not be committed. */
 UNIV_INTERN
 ulint
 row_merge_rename_indexes(

row/row0merge.c

@@ -1726,7 +1726,9 @@ run_again:
 }
 
 /*************************************************************************
-Drop an index from the InnoDB system tables. */
+Drop an index from the InnoDB system tables.  The data dictionary must
+have been locked exclusively by the caller, because the transaction
+will not be committed. */
 UNIV_INTERN
 void
 row_merge_drop_index(
@@ -1736,7 +1738,6 @@ row_merge_drop_index(
 	trx_t*		trx)	/* in: transaction handle */
 {
 	ulint		err;
-	ibool		dict_lock = FALSE;
 	pars_info_t*	info = pars_info_create();
 
 	/* We use the private SQL parser of Innobase to generate the
@@ -1760,10 +1761,7 @@ row_merge_drop_index(
 	trx_start_if_not_started(trx);
 	trx->op_info = "dropping index";
 
-	if (trx->dict_operation_lock_mode == 0) {
-		row_mysql_lock_data_dictionary(trx);
-		dict_lock = TRUE;
-	}
+	ut_a(trx->dict_operation_lock_mode == RW_X_LATCH);
 
 	err = que_eval_sql(info, str1, FALSE, trx);
 
@@ -1775,16 +1773,14 @@ row_merge_drop_index(
 	dict_table_replace_index_in_foreign_list(table, index);
 	dict_index_remove_from_cache(table, index);
 
-	if (dict_lock) {
-		row_mysql_unlock_data_dictionary(trx);
-	}
-
 	trx->op_info = "";
 }
 
 /*************************************************************************
-Drop those indexes which were created before an error occurred
-when building an index. */
+Drop those indexes which were created before an error occurred when
+building an index.  The data dictionary must have been locked
+exclusively by the caller, because the transaction will not be
+committed. */
 UNIV_INTERN
 void
 row_merge_drop_indexes(
@@ -1965,7 +1961,9 @@ row_merge_create_temporary_table(
 }
 
 /*************************************************************************
-Rename the temporary indexes in the dictionary to permanent ones. */
+Rename the temporary indexes in the dictionary to permanent ones.  The
+data dictionary must have been locked exclusively by the caller,
+because the transaction will not be committed. */
 UNIV_INTERN
 ulint
 row_merge_rename_indexes(
@@ -1974,7 +1972,6 @@ row_merge_rename_indexes(
 	trx_t*		trx,		/* in/out: transaction */
 	dict_table_t*	table)		/* in/out: table with new indexes */
 {
-	ibool		dict_lock = FALSE;
 	ulint		err = DB_SUCCESS;
 	pars_info_t*	info = pars_info_create();
 
@@ -1992,18 +1989,14 @@ row_merge_rename_indexes(
 		"WHERE TABLE_ID = :tableid AND SUBSTR(NAME,0,1)='\377';\n"
 		"END;\n";
 
-	ut_ad(table && trx);
+	ut_ad(table)
+	ut_ad(trx);
+	ut_a(trx->dict_operation_lock_mode == RW_X_LATCH);
 
-	trx_start_if_not_started(trx);
 	trx->op_info = "renaming indexes";
 
 	pars_info_add_dulint_literal(info, "tableid", table->id);
 
-	if (trx->dict_operation_lock_mode == 0) {
-		row_mysql_lock_data_dictionary(trx);
-		dict_lock = TRUE;
-	}
-
 	err = que_eval_sql(info, rename_indexes, FALSE, trx);
 
 	if (err == DB_SUCCESS) {
@@ -2016,17 +2009,15 @@ row_merge_rename_indexes(
 		} while (index);
 	}
 
-	if (dict_lock) {
-		row_mysql_unlock_data_dictionary(trx);
-	}
-
 	trx->op_info = "";
 
 	return(err);
 }
 
 /*************************************************************************
-Rename the tables in the data dictionary. */
+Rename the tables in the data dictionary.  The data dictionary must
+have been locked exclusively by the caller, because the transaction
+will not be committed. */
 UNIV_INTERN
 ulint
 row_merge_rename_tables(
@@ -2047,8 +2038,9 @@ row_merge_rename_tables(
 	ut_ad(old_table != new_table);
 	ut_ad(mutex_own(&dict_sys->mutex));
 
+	ut_a(trx->dict_operation_lock_mode == RW_X_LATCH);
+
 	trx->op_info = "renaming tables";
-	trx_start_if_not_started(trx);
 
 	/* We use the private SQL parser of Innobase to generate the query
 	graphs needed in updating the dictionary data in system tables. */

row/row0mysql.c

@@ -3450,8 +3450,7 @@ loop:
 		}
 
 		err = row_drop_table_for_mysql(table_name, trx, TRUE);
-
-		mem_free(table_name);
+		trx_commit_for_mysql(trx);
 
 		if (err != DB_SUCCESS) {
 			fputs("InnoDB: DROP DATABASE ", stderr);
@@ -3460,8 +3459,11 @@ loop:
 				(ulint) err);
 			ut_print_name(stderr, trx, TRUE, table_name);
 			putc('\n', stderr);
+			mem_free(table_name);
 			break;
 		}
+
+		mem_free(table_name);
 	}
 
 	if (err == DB_SUCCESS) {