MDEV-22790 Race between btr_page_mtr_lock() dropping AHI on the same block

This race condition was introduced by
commit ad6171b91cac33e70bb28fa6865488b2c65e858c (MDEV-22456).

In the observed case, two threads were executing
btr_search_drop_page_hash_index() on the same block,
to free a stale entry that was attached to a dropped index.
Both threads were holding an S latch on the block.

We must prevent the double-free of block->index by holding
block->lock in exclusive mode.

btr_search_guess_on_hash(): Do not invoke
btr_search_drop_page_hash_index(block) to get rid of
stale entries, because we are not necessarily holding
an exclusive block->lock here.

buf_defer_drop_ahi(): New function, to safely drop stale
entries in buf_page_mtr_lock(). We will skip the call to
btr_search_drop_page_hash_index(block) when only requesting
bufferfixing (no page latch), because in that case, we should
not be accessing the adaptive hash index, and we might get
a deadlock if we acquired the page latch.

2 files changed, 41 insertions, 2 deletions

diff --git a/storage/innobase/btr/btr0sea.cc b/storage/innobase/btr/btr0sea.cc
index ff1241740a4..491a3e375d3 100644
--- a/storage/innobase/btr/btr0sea.cc
+++ b/storage/innobase/btr/btr0sea.cc
@@ -989,7 +989,6 @@ fail:
 
 		buf_block_dbg_add_level(block, SYNC_TREE_NODE_FROM_HASH);
 		if (UNIV_UNLIKELY(fail)) {
-			btr_search_drop_page_hash_index(block);
 			goto fail_and_release_page;
 		}
 	} else if (UNIV_UNLIKELY(index != block->index
diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc
index 4b78cb38b42..802e67de1b0 100644
--- a/storage/innobase/buf/buf0buf.cc
+++ b/storage/innobase/buf/buf0buf.cc
@@ -4125,6 +4125,46 @@ buf_wait_for_read(
 	}
 }
 
+#ifdef BTR_CUR_HASH_ADAPT
+/** If a stale adaptive hash index exists on the block, drop it.
+Multiple executions of btr_search_drop_page_hash_index() on the
+same block must be prevented by exclusive page latch. */
+ATTRIBUTE_COLD
+static void buf_defer_drop_ahi(buf_block_t *block, mtr_memo_type_t fix_type)
+{
+  switch (fix_type) {
+  case MTR_MEMO_BUF_FIX:
+    /* We do not drop the adaptive hash index, because safely doing
+    so would require acquiring block->lock, and that is not safe
+    to acquire in some RW_NO_LATCH access paths. Those code paths
+    should have no business accessing the adaptive hash index anyway. */
+    break;
+  case MTR_MEMO_PAGE_S_FIX:
+    /* Temporarily release our S-latch. */
+    rw_lock_s_unlock(&block->lock);
+    rw_lock_x_lock(&block->lock);
+    if (dict_index_t *index= block->index)
+      if (index->freed())
+        btr_search_drop_page_hash_index(block);
+    rw_lock_x_unlock(&block->lock);
+    rw_lock_s_lock(&block->lock);
+    break;
+  case MTR_MEMO_PAGE_SX_FIX:
+    rw_lock_sx_unlock(&block->lock);
+    rw_lock_x_lock(&block->lock);
+    if (dict_index_t *index= block->index)
+      if (index->freed())
+        btr_search_drop_page_hash_index(block);
+    rw_lock_x_unlock(&block->lock);
+    rw_lock_sx_lock(&block->lock);
+    break;
+  default:
+    ut_ad(fix_type == MTR_MEMO_PAGE_X_FIX);
+    btr_search_drop_page_hash_index(block);
+  }
+}
+#endif /* BTR_CUR_HASH_ADAPT */
+
 /** Lock the page with the given latch type.
 @param[in,out]	block		block to be locked
 @param[in]	rw_latch	RW_S_LATCH, RW_X_LATCH, RW_NO_LATCH
@@ -4163,7 +4203,7 @@ static buf_block_t* buf_page_mtr_lock(buf_block_t *block,
   {
     dict_index_t *index= block->index;
     if (index && index->freed())
-      btr_search_drop_page_hash_index(block);
+      buf_defer_drop_ahi(block, fix_type);
   }
 #endif /* BTR_CUR_HASH_ADAPT */