Bug#13825266 RACE IN LOCK_VALIDATE() WHEN ACCESSING PAGES DIRECTLY

FROM BUFFER POOL

rb://975
approved by: Marko Makela

There is a race in lock_validate() where we try to access a page
without ensuring that the tablespace stays valid during the operation
i.e.: it is not deleted. This patch tries to fix that by using an
existing flag (the flag is renamed to make it's name more generic
in line with it's new use).

storage/innodb_plugin/ChangeLog

@@ -1,3 +1,10 @@
+2012-03-15	The InnoDB Team
+
+	* fil/fil0fil.c, ibuf/ibuf0ibuf.c, include/fil0fil.h,
+	lock/lock0lock.c:
+	Fix Bug#13825266 RACE IN LOCK_VALIDATE() WHEN ACCESSING PAGES
+	DIRECTLY FROM BUFFER POOL
+
 2012-03-15	The InnoDB Team
 
 	* handler/ha_innodb.cc:

storage/innodb_plugin/fil/fil0fil.c

@@ -175,7 +175,7 @@ struct fil_space_struct {
 				.ibd file of tablespace and want to
 				stop temporarily posting of new i/o
 				requests on the file */
-	ibool		stop_ibuf_merges;
+	ibool		stop_new_ops;
 				/*!< we set this TRUE when we start
 				deleting a single-table tablespace */
 	ibool		is_being_deleted;
@@ -200,12 +200,13 @@ struct fil_space_struct {
 	ulint		n_pending_flushes; /*!< this is positive when flushing
 				the tablespace to disk; dropping of the
 				tablespace is forbidden if this is positive */
-	ulint		n_pending_ibuf_merges;/*!< this is positive
+	ulint		n_pending_ops;/*!< this is positive when we
-				when merging insert buffer entries to
+				have pending operations against this
-				a page so that we may need to access
+				tablespace. The pending operations can
-				the ibuf bitmap page in the
+				be ibuf merges or lock validation code
-				tablespade: dropping of the tablespace
+				trying to read a block.
-				is forbidden if this is positive */
+				Dropping of the tablespace is forbidden
+				if this is positive */
 	hash_node_t	hash;	/*!< hash chain node */
 	hash_node_t	name_hash;/*!< hash chain the name_hash table */
 #ifndef UNIV_HOTBACKUP
@@ -1236,7 +1237,7 @@ try_again:
 	}
 
 	space->stop_ios = FALSE;
-	space->stop_ibuf_merges = FALSE;
+	space->stop_new_ops = FALSE;
 	space->is_being_deleted = FALSE;
 	space->purpose = purpose;
 	space->size = 0;
@@ -1245,7 +1246,7 @@ try_again:
 	space->n_reserved_extents = 0;
 
 	space->n_pending_flushes = 0;
-	space->n_pending_ibuf_merges = 0;
+	space->n_pending_ops = 0;
 
 	UT_LIST_INIT(space->chain);
 	space->magic_n = FIL_SPACE_MAGIC_N;
@@ -1836,13 +1837,12 @@ fil_read_flushed_lsn_and_arch_log_no(
 
 #ifndef UNIV_HOTBACKUP
 /*******************************************************************//**
-Increments the count of pending insert buffer page merges, if space is not
+Increments the count of pending operation, if space is not being deleted.
-being deleted.
+@return	TRUE if being deleted, and operation should be skipped */
-@return	TRUE if being deleted, and ibuf merges should be skipped */
 UNIV_INTERN
 ibool
-fil_inc_pending_ibuf_merges(
+fil_inc_pending_ops(
-/*========================*/
+/*================*/
 	ulint	id)	/*!< in: space id */
 {
 	fil_space_t*	space;
@@ -1858,13 +1858,13 @@ fil_inc_pending_ibuf_merges(
 			(ulong) id);
 	}
 
-	if (space == NULL || space->stop_ibuf_merges) {
+	if (space == NULL || space->stop_new_ops) {
 		mutex_exit(&fil_system->mutex);
 
 		return(TRUE);
 	}
 
-	space->n_pending_ibuf_merges++;
+	space->n_pending_ops++;
 
 	mutex_exit(&fil_system->mutex);
 
@@ -1872,11 +1872,11 @@ fil_inc_pending_ibuf_merges(
 }
 
 /*******************************************************************//**
-Decrements the count of pending insert buffer page merges. */
+Decrements the count of pending operations. */
 UNIV_INTERN
 void
-fil_decr_pending_ibuf_merges(
+fil_decr_pending_ops(
-/*=========================*/
+/*=================*/
 	ulint	id)	/*!< in: space id */
 {
 	fil_space_t*	space;
@@ -1887,13 +1887,13 @@ fil_decr_pending_ibuf_merges(
 
 	if (space == NULL) {
 		fprintf(stderr,
-			"InnoDB: Error: decrementing ibuf merge of a"
+			"InnoDB: Error: decrementing pending operation"
-			" dropped tablespace %lu\n",
+			" of a dropped tablespace %lu\n",
 			(ulong) id);
 	}
 
 	if (space != NULL) {
-		space->n_pending_ibuf_merges--;
+		space->n_pending_ops--;
 	}
 
 	mutex_exit(&fil_system->mutex);
@@ -2183,15 +2183,15 @@ fil_delete_tablespace(
 	char*		path;
 
 	ut_a(id != 0);
-stop_ibuf_merges:
+stop_new_ops:
 	mutex_enter(&fil_system->mutex);
 
 	space = fil_space_get_by_id(id);
 
 	if (space != NULL) {
-		space->stop_ibuf_merges = TRUE;
+		space->stop_new_ops = TRUE;
 
-		if (space->n_pending_ibuf_merges == 0) {
+		if (space->n_pending_ops == 0) {
 			mutex_exit(&fil_system->mutex);
 
 			count = 0;
@@ -2205,9 +2205,10 @@ stop_ibuf_merges:
 				ut_print_filename(stderr, space->name);
 				fprintf(stderr, ",\n"
 					"InnoDB: but there are %lu pending"
-					" ibuf merges on it.\n"
+					" operations (most likely ibuf merges)"
+					" on it.\n"
 					"InnoDB: Loop %lu.\n",
-					(ulong) space->n_pending_ibuf_merges,
+					(ulong) space->n_pending_ops,
 					(ulong) count);
 			}
 
@@ -2216,7 +2217,7 @@ stop_ibuf_merges:
 			os_thread_sleep(20000);
 			count++;
 
-			goto stop_ibuf_merges;
+			goto stop_new_ops;
 		}
 	}
 
@@ -2242,7 +2243,7 @@ try_again:
 	}
 
 	ut_a(space);
-	ut_a(space->n_pending_ibuf_merges == 0);
+	ut_a(space->n_pending_ops == 0);
 
 	space->is_being_deleted = TRUE;
 

storage/innodb_plugin/ibuf/ibuf0ibuf.c

@@ -3281,7 +3281,7 @@ ibuf_merge_or_delete_for_page(
 		function. When the counter is > 0, that prevents tablespace
 		from being dropped. */
 
-		tablespace_being_deleted = fil_inc_pending_ibuf_merges(space);
+		tablespace_being_deleted = fil_inc_pending_ops(space);
 
 		if (UNIV_UNLIKELY(tablespace_being_deleted)) {
 			/* Do not try to read the bitmap page from space;
@@ -3305,7 +3305,7 @@ ibuf_merge_or_delete_for_page(
 				mtr_commit(&mtr);
 
 				if (!tablespace_being_deleted) {
-					fil_decr_pending_ibuf_merges(space);
+					fil_decr_pending_ops(space);
 				}
 
 				return;
@@ -3537,7 +3537,7 @@ reset_bit:
 
 	if (update_ibuf_bitmap && !tablespace_being_deleted) {
 
-		fil_decr_pending_ibuf_merges(space);
+		fil_decr_pending_ops(space);
 	}
 
 	ibuf_exit();

storage/innodb_plugin/include/fil0fil.h

@@ -340,20 +340,19 @@ fil_read_flushed_lsn_and_arch_log_no(
 	ib_uint64_t*	min_flushed_lsn,	/*!< in/out: */
 	ib_uint64_t*	max_flushed_lsn);	/*!< in/out: */
 /*******************************************************************//**
-Increments the count of pending insert buffer page merges, if space is not
+Increments the count of pending operation, if space is not being deleted.
-being deleted.
+@return	TRUE if being deleted, and operation should be skipped */
-@return	TRUE if being deleted, and ibuf merges should be skipped */
 UNIV_INTERN
 ibool
-fil_inc_pending_ibuf_merges(
+fil_inc_pending_ops(
-/*========================*/
+/*================*/
 	ulint	id);	/*!< in: space id */
 /*******************************************************************//**
-Decrements the count of pending insert buffer page merges. */
+Decrements the count of pending operations. */
 UNIV_INTERN
 void
-fil_decr_pending_ibuf_merges(
+fil_decr_pending_ops(
-/*=========================*/
+/*=================*/
 	ulint	id);	/*!< in: space id */
 #endif /* !UNIV_HOTBACKUP */
 /*******************************************************************//**

storage/innodb_plugin/lock/lock0lock.c

@@ -5040,9 +5040,17 @@ lock_validate(void)
 
 			lock_mutex_exit_kernel();
 
-			lock_rec_validate_page(space,
+			/* Make sure that the tablespace is not
-					       fil_space_get_zip_size(space),
+			deleted while we are trying to access
-					       page_no);
+			the page. */
+			if (!fil_inc_pending_ops(space)) {
+				lock_rec_validate_page(
+					space,
+					fil_space_get_zip_size(space),
+					page_no);
+
+				fil_decr_pending_ops(space);
+			}
 
 			lock_mutex_enter_kernel();