Bug#12584374 LOCK_VALIDATE TRIPS ASSERTION !BLOCK->PAGE.FILE_PAGE_WAS_FREE

Fix a deadlock in the initial patch. lock_validate() must not hold the
lock system mutex while s-latching a block, because some functions,
such as lock_rec_convert_impl_to_expl(), may be already holding an x-latch
on the block that lock_validate() is interested in while attempting to
acquire the lock system mutex.

This deadlock was not caught by UNIV_SYNC_DEBUG because of
buf_block_dbg_add_level(block, SYNC_NO_ORDER_CHECK).

1 files changed, 26 insertions, 29 deletions

diff --git a/storage/innobase/lock/lock0lock.c b/storage/innobase/lock/lock0lock.c
index 7d3af6ced07..714107304f7 100644
--- a/storage/innobase/lock/lock0lock.c
+++ b/storage/innobase/lock/lock0lock.c
@@ -352,7 +352,6 @@ ibool
 lock_validate(void);
 /*===============*/
 
-# ifdef UNIV_DEBUG_LOCK_VALIDATE
 /*********************************************************************//**
 Validates the record lock queues on a page.
 @return	TRUE if ok */
@@ -362,7 +361,6 @@ lock_rec_validate_page(
 /*===================*/
 	const buf_block_t*	block)	/*!< in: buffer block */
 	__attribute__((nonnull, warn_unused_result));
-# endif /* UNIV_DEBUG_LOCK_VALIDATE */
 #endif /* UNIV_DEBUG */
 
 /* The lock system */
@@ -4865,10 +4863,10 @@ lock_rec_queue_validate(
 /*********************************************************************//**
 Validates the record lock queues on a page.
 @return	TRUE if ok */
-static __attribute__((nonnull, warn_unused_result))
+static
 ibool
-lock_rec_validate_page_low(
-/*=======================*/
+lock_rec_validate_page(
+/*===================*/
 	const buf_block_t*	block)	/*!< in: buffer block */
 {
 	const lock_t*	lock;
@@ -4881,9 +4879,10 @@ lock_rec_validate_page_low(
 	ulint*		offsets		= offsets_;
 	rec_offs_init(offsets_);
 
-	ut_ad(mutex_own(&kernel_mutex));
+	ut_ad(!mutex_own(&kernel_mutex));
 	ut_ad(buf_block_get_state(block) == BUF_BLOCK_FILE_PAGE);
 
+	lock_mutex_enter_kernel();
 loop:
 	lock = lock_rec_get_first_on_page_addr(buf_block_get_space(block),
 					       buf_block_get_page_no(block));
@@ -4892,6 +4891,10 @@ loop:
 		goto function_exit;
 	}
 
+#if defined UNIV_DEBUG_FILE_ACCESSES || defined UNIV_DEBUG
+	ut_a(!block->page.file_page_was_freed);
+#endif
+
 	for (i = 0; i < nth_lock; i++) {
 
 		lock = lock_rec_get_next_on_page_const(lock);
@@ -4950,32 +4953,14 @@ loop:
 	goto loop;
 
 function_exit:
+	lock_mutex_exit_kernel();
+
 	if (UNIV_LIKELY_NULL(heap)) {
 		mem_heap_free(heap);
 	}
 	return(TRUE);
 }
 
-#ifdef UNIV_DEBUG_LOCK_VALIDATE
-/*********************************************************************//**
-Validates the record lock queues on a page.
-@return	TRUE if ok */
-static
-ibool
-lock_rec_validate_page(
-/*===================*/
-	const buf_block_t*	block)	/*!< in: buffer block */
-{
-	ibool	valid;
-
-	lock_mutex_enter_kernel();
-	valid = lock_rec_validate_page_low(block);
-	lock_mutex_exit_kernel();
-
-	return(valid);
-}
-#endif /* UNIV_DEBUG_LOCK_VALIDATE */
-
 /*********************************************************************//**
 Validates the lock system.
 @return	TRUE if ok */
@@ -5041,16 +5026,28 @@ lock_validate(void)
 				break;
 			}
 
+			lock_mutex_exit_kernel();
+
+			/* The lock and the block that it is referring
+			to may be freed at this point. We pass
+			BUF_GET_POSSIBLY_FREED to skip a debug check.
+			If the lock exists in lock_rec_validate_page()
+			we assert !block->page.file_page_was_freed. */
+
 			mtr_start(&mtr);
-			block = buf_page_get(
+			block = buf_page_get_gen(
 				space, fil_space_get_zip_size(space),
-				page_no, RW_X_LATCH, &mtr);
+				page_no, RW_X_LATCH, NULL,
+				BUF_GET_POSSIBLY_FREED,
+				__FILE__, __LINE__, &mtr);
 			buf_block_dbg_add_level(block, SYNC_NO_ORDER_CHECK);
 
-			ut_ad(lock_rec_validate_page_low(block));
+			ut_ad(lock_rec_validate_page(block));
 			mtr_commit(&mtr);
 
 			limit++;
+
+			lock_mutex_enter_kernel();
 		}
 	}