branches/5.1 issue# 4

Fixed a timing hole where a thread dropping an index can free the
in-memory index struct while another thread is still using
that structure to remove entries from adaptive hash index belonging
to one of the pages that belongs to the index being dropped.

The fix is to have a reference counter in the index struct and to
wait for this counter to drop to zero beforing freeing the struct.

Reviewed by: Heikki

btr/btr0sea.c

@@ -161,6 +161,7 @@ btr_search_info_create(
 	info->magic_n = BTR_SEARCH_MAGIC_N;
 #endif /* UNIV_DEBUG */
 
+	info->ref_count = 0;
 	info->root_guess = NULL;
 
 	info->hash_analysis = 0;
@@ -184,6 +185,32 @@ btr_search_info_create(
 	return(info);
 }
 
+/*********************************************************************
+Returns the value of ref_count. The value is protected by
+btr_search_latch. */
+UNIV_INLINE
+ulint
+btr_search_info_get_ref_count(
+/*==========================*/
+				/* out: ref_count value. */
+	btr_search_t*   info)	/* in: search info. */
+{
+	ulint ret;
+
+	ut_ad(info);
+
+#ifdef UNIV_SYNC_DEBUG
+	ut_ad(!rw_lock_own(&btr_search_latch, RW_LOCK_SHARED));
+	ut_ad(!rw_lock_own(&btr_search_latch, RW_LOCK_EX));
+#endif /* UNIV_SYNC_DEBUG */
+
+	rw_lock_s_lock(&btr_search_latch);
+	ret = info->ref_count;
+	rw_lock_s_unlock(&btr_search_latch);
+
+	return(ret);
+}
+
 /*************************************************************************
 Updates the search info of an index about hash successes. NOTE that info
 is NOT protected by any semaphore, to save CPU time! Do not assume its fields
@@ -1022,8 +1049,12 @@ next_rec:
 		ha_remove_all_nodes_to_page(table, folds[i], page);
 	}
 
+	ut_a(index->search_info->ref_count > 0);
+	index->search_info->ref_count--;
+
 	block->is_hashed = FALSE;
 	block->index = NULL;
+	
 cleanup:
 	if (UNIV_UNLIKELY(block->n_pointers)) {
 		/* Corruption */
@@ -1244,6 +1275,15 @@ btr_search_build_page_hash_index(
 		goto exit_func;
 	}
 
+	/* This counter is decremented every time we drop page
+	hash index entries and is incremented here. Since we can
+	rebuild hash index for a page that is already hashed, we
+	have to take care not to increment the counter in that
+	case. */
+	if (!block->is_hashed) {
+		index->search_info->ref_count++;
+	}
+
 	block->is_hashed = TRUE;
 	block->n_hash_helps = 0;
 

dict/dict0dict.c

@@ -1394,12 +1394,59 @@ dict_index_remove_from_cache(
 	dict_index_t*	index)	/* in, own: index */
 {
 	ulint		size;
+	ulint		retries = 0;
+	btr_search_t*	info;
 
 	ut_ad(table && index);
 	ut_ad(table->magic_n == DICT_TABLE_MAGIC_N);
 	ut_ad(index->magic_n == DICT_INDEX_MAGIC_N);
 	ut_ad(mutex_own(&(dict_sys->mutex)));
 
+	/* We always create search info whether or not adaptive
+	hash index is enabled or not. */
+	info = index->search_info;
+	ut_ad(info);
+
+	/* We are not allowed to free the in-memory index struct
+ 	dict_index_t until all entries in the adaptive hash index
+	that point to any of the page belonging to his b-tree index
+	are dropped. This is so because dropping of these entries
+	require access to dict_index_t struct. To avoid such scenario
+	We keep a count of number of such pages in the search_info and
+	only free the dict_index_t struct when this count drops to
+	zero. */
+
+	for (;;) {
+		ulint ref_count = btr_search_info_get_ref_count(info);
+		if (ref_count == 0) {
+			break;
+		}
+
+		/* Sleep for 10ms before trying again. */
+		os_thread_sleep(10000);
+		++retries;
+
+		if (retries % 500 == 0) {
+			/* No luck after 5 seconds of wait. */
+			fprintf(stderr, "InnoDB: Error: Waited for"
+					" %lu secs for hash index"
+					" ref_count (%lu) to drop"
+					" to 0.\n"
+					"index: \"%s\""
+					" table: \"%s\"\n",
+					retries/100,
+					ref_count,
+					index->name,
+					table->name);
+		}
+
+		/* To avoid a hang here we commit suicide if the
+		ref_count doesn't drop to zero in 600 seconds. */
+		if (retries >= 60000) {
+			ut_error;
+		}
+	}
+
 	rw_lock_free(&index->lock);
 
 	/* Remove the index from the list of indexes of the table */

include/btr0sea.h

@@ -40,6 +40,15 @@ btr_search_info_create(
 /*===================*/
 				/* out, own: search info struct */
 	mem_heap_t*	heap);	/* in: heap where created */
+/*********************************************************************
+Returns the value of ref_count. The value is protected by
+btr_search_latch. */
+UNIV_INLINE
+ulint
+btr_search_info_get_ref_count(
+/*==========================*/
+				/* out: ref_count value. */
+	btr_search_t*   info);	/* in: search info. */
 /*************************************************************************
 Updates the search info. */
 UNIV_INLINE
@@ -137,6 +146,13 @@ btr_search_validate(void);
 /* The search info struct in an index */
 
 struct btr_search_struct{
+	ulint	ref_count;	/* Number of blocks in this index tree
+				that have search index built
+				i.e. block->index points to this index.
+				Protected by btr_search_latch except
+				when during initialization in
+				btr_search_info_create(). */
+
 	/* The following fields are not protected by any latch.
 	Unfortunately, this means that they must be aligned to
 	the machine word, i.e., they cannot be turned into bit-fields. */