MDEV-20612 preparation: LockMutexGuard

Let us use the RAII wrapper LockMutexGuard for most operations where
lock_sys.mutex is acquired.

storage/innobase/buf/buf0buf.cc

@@ -2038,9 +2038,8 @@ inline void buf_pool_t::resize()
 			{found, withdraw_started, my_hrtime_coarse()};
 		withdraw_started = current_time;
 
-		lock_sys.mutex_lock();
+		LockMutexGuard g;
 		trx_sys.trx_list.for_each(f);
-		lock_sys.mutex_unlock();
 	}
 
 	if (should_retry_withdraw) {

storage/innobase/gis/gis0sea.cc

@@ -386,11 +386,12 @@ rtr_pcur_getnext_from_path(
 
 			trx_t*		trx = thr_get_trx(
 						btr_cur->rtr_info->thr);
-			lock_sys.mutex_lock();
-			lock_init_prdt_from_mbr(
-				&prdt, &btr_cur->rtr_info->mbr,
-				mode, trx->lock.lock_heap);
-			lock_sys.mutex_unlock();
+			{
+				LockMutexGuard g;
+				lock_init_prdt_from_mbr(
+					&prdt, &btr_cur->rtr_info->mbr,
+					mode, trx->lock.lock_heap);
+			}
 
 			if (rw_latch == RW_NO_LATCH) {
 				block->lock.s_lock();
@@ -1182,18 +1183,15 @@ rtr_check_discard_page(
 		}
 		mysql_mutex_unlock(&rtr_info->rtr_path_mutex);
 
-		if (rtr_info->matches) {
-			mysql_mutex_lock(&rtr_info->matches->rtr_match_mutex);
+		if (auto matches = rtr_info->matches) {
+			mysql_mutex_lock(&matches->rtr_match_mutex);
 
-			if ((&rtr_info->matches->block)->page.id() == id) {
-				if (!rtr_info->matches->matched_recs->empty()) {
-					rtr_info->matches->matched_recs->clear();
-				}
-				ut_ad(rtr_info->matches->matched_recs->empty());
-				rtr_info->matches->valid = false;
+			if (matches->block.page.id() == id) {
+				matches->matched_recs->clear();
+				matches->valid = false;
 			}
 
-			mysql_mutex_unlock(&rtr_info->matches->rtr_match_mutex);
+			mysql_mutex_unlock(&matches->rtr_match_mutex);
 		}
 	}
 

storage/innobase/handler/ha_innodb.cc

@@ -4465,16 +4465,17 @@ static void innobase_kill_query(handlerton*, THD *thd, enum thd_kill_levels)
 #endif /* WITH_WSREP */
     if (trx->lock.wait_lock)
     {
-      lock_sys.mutex_lock();
-      mysql_mutex_lock(&lock_sys.wait_mutex);
-      if (lock_t *lock= trx->lock.wait_lock)
       {
-        trx->mutex_lock();
-        trx->error_state= DB_INTERRUPTED;
-        lock_cancel_waiting_and_release(lock);
-        trx->mutex_unlock();
+        LockMutexGuard g;
+        mysql_mutex_lock(&lock_sys.wait_mutex);
+        if (lock_t *lock= trx->lock.wait_lock)
+        {
+          trx->mutex_lock();
+          trx->error_state= DB_INTERRUPTED;
+          lock_cancel_waiting_and_release(lock);
+          trx->mutex_unlock();
+        }
       }
-      lock_sys.mutex_unlock();
       mysql_mutex_unlock(&lock_sys.wait_mutex);
     }
   }
@@ -18101,11 +18102,10 @@ wsrep_abort_transaction(
 			wsrep_thd_transaction_state_str(victim_thd));
 
 	if (victim_trx) {
-		lock_sys.mutex_lock();
+		LockMutexGuard g;
 		victim_trx->mutex_lock();
 		int rcode= wsrep_innobase_kill_one_trx(bf_thd,
 						       victim_trx, signal);
-		lock_sys.mutex_unlock();
 		victim_trx->mutex_unlock();
 		DBUG_RETURN(rcode);
 	} else {

storage/innobase/ibuf/ibuf0ibuf.cc

@@ -3280,10 +3280,8 @@ ibuf_insert_low(
 		ibuf_mtr_commit(&bitmap_mtr);
 		goto fail_exit;
 	} else {
-		lock_sys.mutex_lock();
-		const auto lock_exists = lock_sys.get_first(page_id);
-		lock_sys.mutex_unlock();
-		if (lock_exists) {
+		LockMutexGuard g;
+		if (lock_sys.get_first(page_id)) {
 			goto commit_exit;
 		}
 	}

storage/innobase/include/trx0trx.h

@@ -485,7 +485,7 @@ struct trx_lock_t
 	/** List of pending trx_t::evict_table() */
 	UT_LIST_BASE_NODE_T(dict_table_t) evicted_tables;
 
-  /** number of record locks; writes are protected by lock_sys.mutex */
+  /** number of record locks; writers use LockGuard or LockMutexGuard */
   ulint n_rec_locks;
 };
 

storage/innobase/lock/lock0prdt.cc

@@ -234,16 +234,13 @@ lock_prdt_has_lock(
 						attached to the new lock */
 	const trx_t*		trx)		/*!< in: transaction */
 {
-	lock_t*		lock;
-
 	lock_sys.mutex_assert_locked();
 	ut_ad((precise_mode & LOCK_MODE_MASK) == LOCK_S
 	      || (precise_mode & LOCK_MODE_MASK) == LOCK_X);
 	ut_ad(!(precise_mode & LOCK_INSERT_INTENTION));
 
-	for (lock = lock_rec_get_first(
-		lock_hash_get(type_mode), id, PRDT_HEAPNO);
-	     lock != NULL;
+	for (lock_t* lock = lock_rec_get_first(lock_hash_get(type_mode), id,
+					       PRDT_HEAPNO); lock;
 	     lock = lock_rec_get_next(PRDT_HEAPNO, lock)) {
 		ut_ad(lock->type_mode & (LOCK_PREDICATE | LOCK_PRDT_PAGE));
 
@@ -291,10 +288,8 @@ lock_prdt_other_has_conflicting(
 					the new lock will be on */
 	const trx_t*		trx)	/*!< in: our transaction */
 {
-	lock_sys.mutex_assert_locked();
-
-	for (lock_t* lock = lock_rec_get_first(
-		lock_hash_get(mode), id, PRDT_HEAPNO);
+	for (lock_t* lock = lock_rec_get_first(lock_hash_get(mode), id,
+					       PRDT_HEAPNO);
 	     lock != NULL;
 	     lock = lock_rec_get_next(PRDT_HEAPNO, lock)) {
 
@@ -388,8 +383,6 @@ lock_prdt_find_on_page(
 {
 	lock_t*	lock;
 
-	lock_sys.mutex_assert_locked();
-
 	for (lock = lock_sys.get_first(*lock_hash_get(type_mode),
 				       block->page.id());
 	     lock != NULL;
@@ -502,83 +495,58 @@ lock_prdt_insert_check_and_lock(
 	lock_prdt_t*	prdt)	/*!< in: Predicates with Minimum Bound
 				Rectangle */
 {
-	ut_ad(block->frame == page_align(rec));
-	ut_ad(!index->table->is_temporary());
-	ut_ad(index->is_spatial());
-
-	trx_t*	trx = thr_get_trx(thr);
-	const page_id_t id{block->page.id()};
-
-	lock_sys.mutex_lock();
-
-	/* Because this code is invoked for a running transaction by
-	the thread that is serving the transaction, it is not necessary
-	to hold trx->mutex here. */
-
-	ut_ad(lock_table_has(trx, index->table, LOCK_IX));
-
-	lock_t*		lock;
-
-	/* Only need to check locks on prdt_hash */
-	lock = lock_rec_get_first(&lock_sys.prdt_hash, id, PRDT_HEAPNO);
-
-	if (lock == NULL) {
-		lock_sys.mutex_unlock();
-
-		/* Update the page max trx id field */
-		page_update_max_trx_id(block, buf_block_get_page_zip(block),
-				       trx->id, mtr);
-
-		return(DB_SUCCESS);
-	}
-
-	ut_ad(lock->type_mode & LOCK_PREDICATE);
-
-	dberr_t		err;
-
-	/* If another transaction has an explicit lock request which locks
-	the predicate, waiting or granted, on the successor, the insert
-	has to wait.
-
-	Similar to GAP lock, we do not consider lock from inserts conflicts
-	with each other */
-
-	const ulint	mode = LOCK_X | LOCK_PREDICATE | LOCK_INSERT_INTENTION;
-
-	const lock_t*	wait_for = lock_prdt_other_has_conflicting(
-		mode, id, prdt, trx);
-
-	if (wait_for != NULL) {
-		rtr_mbr_t*	mbr = prdt_get_mbr_from_prdt(prdt);
-
-		/* Allocate MBR on the lock heap */
-		lock_init_prdt_from_mbr(prdt, mbr, 0, trx->lock.lock_heap);
-
-		/* Note that we may get DB_SUCCESS also here! */
-		trx->mutex_lock();
-
-		err = lock_rec_enqueue_waiting(
+  ut_ad(block->frame == page_align(rec));
+  ut_ad(!index->table->is_temporary());
+  ut_ad(index->is_spatial());
+
+  trx_t *trx= thr_get_trx(thr);
+  const page_id_t id{block->page.id()};
+  dberr_t err= DB_SUCCESS;
+
+  {
+    LockMutexGuard g;
+    /* Because this code is invoked for a running transaction by
+    the thread that is serving the transaction, it is not necessary
+    to hold trx->mutex here. */
+    ut_ad(lock_table_has(trx, index->table, LOCK_IX));
+
+    /* Only need to check locks on prdt_hash */
+    if (ut_d(lock_t *lock=)
+        lock_rec_get_first(&lock_sys.prdt_hash, id, PRDT_HEAPNO))
+    {
+      ut_ad(lock->type_mode & LOCK_PREDICATE);
+
+      /* If another transaction has an explicit lock request which locks
+      the predicate, waiting or granted, on the successor, the insert
+      has to wait.
+
+      Similar to GAP lock, we do not consider lock from inserts conflicts
+      with each other */
+
+      const ulint mode= LOCK_X | LOCK_PREDICATE | LOCK_INSERT_INTENTION;
+      lock_t *c_lock= lock_prdt_other_has_conflicting(mode, id, prdt, trx);
+
+      if (c_lock)
+      {
+        rtr_mbr_t *mbr= prdt_get_mbr_from_prdt(prdt);
+        /* Allocate MBR on the lock heap */
+        lock_init_prdt_from_mbr(prdt, mbr, 0, trx->lock.lock_heap);
+        trx->mutex_lock();
+        err= lock_rec_enqueue_waiting(
 #ifdef WITH_WSREP
-			NULL, /* FIXME: replicate SPATIAL INDEX locks */
+          c_lock,
 #endif
-			LOCK_X | LOCK_PREDICATE | LOCK_INSERT_INTENTION,
-			id, block->frame, PRDT_HEAPNO, index, thr, prdt);
-
-		trx->mutex_unlock();
-	} else {
-		err = DB_SUCCESS;
-	}
+          mode, id, block->frame, PRDT_HEAPNO, index, thr, prdt);
+        trx->mutex_unlock();
+      }
+    }
+  }
 
-	lock_sys.mutex_unlock();
+  if (err == DB_SUCCESS)
+    /* Update the page max trx id field */
+    page_update_max_trx_id(block, buf_block_get_page_zip(block), trx->id, mtr);
 
-	if (err == DB_SUCCESS) {
-		/* Update the page max trx id field */
-		page_update_max_trx_id(block,
-				       buf_block_get_page_zip(block),
-				       trx->id, mtr);
-	}
-
-	return(err);
+  return err;
 }
 
 /**************************************************************//**
@@ -593,7 +561,7 @@ lock_prdt_update_parent(
         lock_prdt_t*	right_prdt,	/*!< in: MBR on the new page */
 	const page_id_t	page_id)	/*!< in: parent page */
 {
-	lock_sys.mutex_lock();
+	LockMutexGuard g;
 
 	/* Get all locks in parent */
 	for (lock_t *lock = lock_sys.get_first_prdt(page_id);
@@ -630,8 +598,6 @@ lock_prdt_update_parent(
 					       lock_prdt, false);
 		}
 	}
-
-	lock_sys.mutex_unlock();
 }
 
 /**************************************************************//**
@@ -694,15 +660,13 @@ lock_prdt_update_split(
 	lock_prdt_t*	new_prdt,	/*!< in: MBR on the new page */
 	const page_id_t	page_id)	/*!< in: page number */
 {
-	lock_sys.mutex_lock();
+	LockMutexGuard g;
 
 	lock_prdt_update_split_low(new_block, prdt, new_prdt,
 				   page_id, LOCK_PREDICATE);
 
 	lock_prdt_update_split_low(new_block, NULL, NULL,
 				   page_id, LOCK_PRDT_PAGE);
-
-	lock_sys.mutex_unlock();
 }
 
 /*********************************************************************//**
@@ -768,7 +732,7 @@ lock_prdt_lock(
 	index record, and this would not have been possible if another active
 	transaction had modified this secondary index record. */
 
-	lock_sys.mutex_lock();
+	LockMutexGuard g;
 
 	const unsigned	prdt_mode = type_mode | mode;
 	lock_t*		lock = lock_sys.get_first(hash, id);
@@ -831,8 +795,6 @@ lock_prdt_lock(
 		}
 	}
 
-	lock_sys.mutex_unlock();
-
 	if (status == LOCK_REC_SUCCESS_CREATED && type_mode == LOCK_PREDICATE) {
 		/* Append the predicate in the lock record */
 		lock_prdt_set_prdt(lock, prdt);
@@ -861,7 +823,7 @@ lock_place_prdt_page_lock(
 	index record, and this would not have been possible if another active
 	transaction had modified this secondary index record. */
 
-	lock_sys.mutex_lock();
+	LockMutexGuard g;
 
 	const lock_t*	lock = lock_sys.get_first_prdt_page(page_id);
 	const ulint	mode = LOCK_S | LOCK_PRDT_PAGE;
@@ -891,8 +853,6 @@ lock_place_prdt_page_lock(
 #endif /* PRDT_DIAG */
 	}
 
-	lock_sys.mutex_unlock();
-
 	return(DB_SUCCESS);
 }
 
@@ -902,15 +862,9 @@ lock_place_prdt_page_lock(
 @return	true if there is none */
 bool lock_test_prdt_page_lock(const trx_t *trx, const page_id_t page_id)
 {
-	lock_t*		lock;
-
-	lock_sys.mutex_lock();
-
-	lock = lock_sys.get_first_prdt_page(page_id);
-
-	lock_sys.mutex_unlock();
-
-	return(!lock || trx == lock->trx);
+  LockMutexGuard g;
+  lock_t *lock= lock_sys.get_first_prdt_page(page_id);
+  return !lock || trx == lock->trx;
 }
 
 /*************************************************************//**
@@ -923,7 +877,7 @@ lock_prdt_rec_move(
 						the receiving record */
 	const page_id_t		donator)	/*!< in: target page */
 {
-	lock_sys.mutex_lock();
+	LockMutexGuard g;
 
 	for (lock_t *lock = lock_rec_get_first(&lock_sys.prdt_hash,
 					       donator, PRDT_HEAPNO);
@@ -942,8 +896,6 @@ lock_prdt_rec_move(
 			type_mode, receiver, lock->index, lock->trx,
 			lock_prdt, false);
 	}
-
-	lock_sys.mutex_unlock();
 }
 
 /** Removes predicate lock objects set on an index page which is discarded.
@@ -952,18 +904,12 @@ lock_prdt_rec_move(
 void
 lock_prdt_page_free_from_discard(const page_id_t id, hash_table_t *lock_hash)
 {
-	lock_t*	lock;
-	lock_t*	next_lock;
-
-	lock_sys.mutex_assert_locked();
-
-	lock = lock_sys.get_first(*lock_hash, id);
-
-	while (lock != NULL) {
-		next_lock = lock_rec_get_next_on_page(lock);
-
-		lock_rec_discard(lock);
-
-		lock = next_lock;
-	}
+  lock_sys.mutex_assert_locked();
+
+  for (lock_t *lock= lock_sys.get_first(*lock_hash, id), *next; lock;
+       lock= next)
+  {
+    next= lock_rec_get_next_on_page(lock);
+    lock_rec_discard(lock);
+  }
 }

storage/innobase/row/row0ins.cc

@@ -707,11 +707,12 @@ row_ins_foreign_trx_print(
 
 	ut_ad(!srv_read_only_mode);
 
-	lock_sys.mutex_lock();
-	n_rec_locks = trx->lock.n_rec_locks;
-	n_trx_locks = UT_LIST_GET_LEN(trx->lock.trx_locks);
-	heap_size = mem_heap_get_size(trx->lock.lock_heap);
-	lock_sys.mutex_unlock();
+	{
+		LockMutexGuard g;
+		n_rec_locks = trx->lock.n_rec_locks;
+		n_trx_locks = UT_LIST_GET_LEN(trx->lock.trx_locks);
+		heap_size = mem_heap_get_size(trx->lock.lock_heap);
+	}
 
 	mysql_mutex_lock(&dict_foreign_err_mutex);
 	rewind(dict_foreign_err_file);

storage/innobase/row/row0mysql.cc

@@ -2613,9 +2613,10 @@ row_drop_tables_for_mysql_in_background(void)
 	}
 
 	if (!srv_fast_shutdown && !trx_sys.any_active_transactions()) {
-		lock_sys.mutex_lock();
-		skip = UT_LIST_GET_LEN(table->locks) != 0;
-		lock_sys.mutex_unlock();
+		{
+			LockMutexGuard g;
+			skip = UT_LIST_GET_LEN(table->locks) != 0;
+		}
 		if (skip) {
 			/* We cannot drop tables that are locked by XA
 			PREPARE transactions. */

storage/innobase/trx/trx0i_s.cc

@@ -1181,7 +1181,7 @@ static void fetch_data_into_cache_low(trx_i_s_cache_t *cache, const trx_t *trx)
 
 static void fetch_data_into_cache(trx_i_s_cache_t *cache)
 {
-  lock_sys.mutex_assert_locked();
+  LockMutexGuard g;
   trx_i_s_cache_clear(cache);
 
   /* Capture the state of transactions */
@@ -1211,10 +1211,7 @@ trx_i_s_possibly_fetch_data_into_cache(
 	}
 
 	/* We need to read trx_sys and record/table lock queues */
-
-	lock_sys.mutex_lock();
 	fetch_data_into_cache(cache);
-	lock_sys.mutex_unlock();
 
 	/* update cache last read time */
 	cache->last_read = my_interval_timer();

storage/innobase/trx/trx0trx.cc

@@ -1257,12 +1257,12 @@ trx_update_mod_tables_timestamp(
 		/* recheck while holding the mutex that blocks
 		table->acquire() */
 		dict_sys.mutex_lock();
-		lock_sys.mutex_lock();
-		const bool do_evict = !table->get_ref_count()
-			&& !UT_LIST_GET_LEN(table->locks);
-		lock_sys.mutex_unlock();
-		if (do_evict) {
-			dict_sys.remove(table, true);
+		{
+			LockMutexGuard g;
+			if (!table->get_ref_count()
+			    && !UT_LIST_GET_LEN(table->locks)) {
+				dict_sys.remove(table, true);
+			}
 		}
 		dict_sys.mutex_unlock();
 #endif
@@ -1862,18 +1862,15 @@ trx_print(
 	ulint		max_query_len)	/*!< in: max query length to print,
 					or 0 to use the default max length */
 {
-	ulint	n_rec_locks;
-	ulint	n_trx_locks;
-	ulint	heap_size;
-
-	lock_sys.mutex_lock();
-	n_rec_locks = trx->lock.n_rec_locks;
-	n_trx_locks = UT_LIST_GET_LEN(trx->lock.trx_locks);
-	heap_size = mem_heap_get_size(trx->lock.lock_heap);
-	lock_sys.mutex_unlock();
+  ulint n_rec_locks, n_trx_locks, heap_size;
+  {
+    LockMutexGuard g;
+    n_rec_locks= trx->lock.n_rec_locks;
+    n_trx_locks= UT_LIST_GET_LEN(trx->lock.trx_locks);
+    heap_size= mem_heap_get_size(trx->lock.lock_heap);
+  }
 
-	trx_print_low(f, trx, max_query_len,
-		      n_rec_locks, n_trx_locks, heap_size);
+  trx_print_low(f, trx, max_query_len, n_rec_locks, n_trx_locks, heap_size);
 }
 
 /** Prepare a transaction.