MDEV-39344: trx_disconnect_prepared() uses wrong mutex

trx_t::disconnect_prepared(): Replaces trx_disconnect_prepared().
Protect the data members with trx_t::mutex.

trx_sys.trx_list: Use a plain ilist<trx_t> that is protected
by lock_sys.latch, which was already protecting some traversal of
trx_sys.trx_list. All traversal will now use range for, instead of
callback functions.

fetch_data_into_cache_low(): Return whether we ran out of memory.

fetch_data_into_cache(): Reduce the hold time of lock_sys.latch
and do not wrongly reset the cache->is_truncated flag at the end.

trx_sys_t::clone_oldest_view(): Protect the list traversal with
shared lock_sys.latch.

trx_sys_t::register_trx(), trx_sys_t::deregister_trx():
Protect trx_sys.trx_list with an exclusive lock_sys.latch.

Author: dr-m

storage/innobase/buf/buf0buf.cc

@@ -1534,42 +1534,6 @@ inline void buf_pool_t::page_hash_table::write_unlock_all() noexcept
 }
 
 
-namespace
-{
-
-struct find_interesting_trx
-{
-  void operator()(const trx_t &trx)
-  {
-    if (!trx.is_started())
-      return;
-    if (trx.mysql_thd == nullptr)
-      return;
-    if (withdraw_started <= trx.start_time_micro)
-      return;
-
-    if (!found)
-    {
-      sql_print_warning("InnoDB: The following trx might hold "
-                    "the blocks in buffer pool to "
-                    "be withdrawn. Buffer pool "
-                    "resizing can complete only "
-                    "after all the transactions "
-                    "below release the blocks.");
-      found= true;
-    }
-
-    lock_trx_print_wait_and_mvcc_state(stderr, &trx, current_time);
-  }
-
-  bool &found;
-  /** microsecond_interval_timer() */
-  const ulonglong withdraw_started;
-  const my_hrtime_t current_time;
-};
-
-} // namespace
-
 /** Resize from srv_buf_pool_old_size to srv_buf_pool_size. */
 inline void buf_pool_t::resize()
 {
@@ -1656,15 +1620,37 @@ inline void buf_pool_t::resize()
 			message_interval *= 2;
 		}
 
-		bool found= false;
-		find_interesting_trx f
-			{found, withdraw_started, my_hrtime_coarse()};
 		withdraw_started = current_time;
+		const my_hrtime_t current_hrtime{my_hrtime_coarse()};
+		bool found{false};
 
 		/* This is going to exceed the maximum size of a
 		memory transaction. */
 		LockMutexGuard g{SRW_LOCK_CALL};
-		trx_sys.trx_list.for_each(f);
+		for (const trx_t& trx: trx_sys.trx_list) {
+			if (!trx.is_started() || !trx.mysql_thd
+			    || withdraw_started <= trx.start_time_micro) {
+				continue;
+			}
+
+			if (!found) {
+				sql_print_warning("InnoDB: The following "
+						  "trx might hold "
+						  "the blocks in "
+						  "buffer pool to "
+						  "be withdrawn. "
+						  "Buffer pool "
+						  "resizing can "
+						  "complete only "
+						  "after all "
+						  "the transactions "
+						  "below release the blocks.");
+				found = true;
+			}
+
+			lock_trx_print_wait_and_mvcc_state(stderr, &trx,
+							   current_hrtime);
+		}
 	}
 
 	if (should_retry_withdraw) {

storage/innobase/handler/ha_innodb.cc

@@ -2953,7 +2953,7 @@ static int innobase_close_connection(handlerton *, THD *thd) noexcept
     case TRX_STATE_PREPARED:
       if (trx->has_logged_persistent())
       {
-        trx_disconnect_prepared(trx);
+        trx->disconnect_prepared();
         return 0;
       }
       /* fall through */

storage/innobase/include/trx0sys.h

@@ -781,58 +781,6 @@ class rw_trx_hash_t
   }
 };
 
-class thread_safe_trx_ilist_t
-{
-public:
-  void create() { mysql_mutex_init(trx_sys_mutex_key, &mutex, nullptr); }
-  void close() { mysql_mutex_destroy(&mutex); }
-
-  bool empty() const
-  {
-    mysql_mutex_lock(&mutex);
-    auto result= trx_list.empty();
-    mysql_mutex_unlock(&mutex);
-    return result;
-  }
-
-  void push_front(trx_t &trx)
-  {
-    mysql_mutex_lock(&mutex);
-    trx_list.push_front(trx);
-    mysql_mutex_unlock(&mutex);
-  }
-
-  void remove(trx_t &trx)
-  {
-    mysql_mutex_lock(&mutex);
-    trx_list.remove(trx);
-    mysql_mutex_unlock(&mutex);
-  }
-
-  template <typename Callable> void for_each(Callable &&callback) const
-  {
-    mysql_mutex_lock(&mutex);
-    for (const auto &trx : trx_list)
-      callback(trx);
-    mysql_mutex_unlock(&mutex);
-  }
-
-  template <typename Callable> void for_each(Callable &&callback)
-  {
-    mysql_mutex_lock(&mutex);
-    for (auto &trx : trx_list)
-      callback(trx);
-    mysql_mutex_unlock(&mutex);
-  }
-
-  void freeze() const { mysql_mutex_lock(&mutex); }
-  void unfreeze() const { mysql_mutex_unlock(&mutex); }
-
-private:
-  alignas(CPU_LEVEL1_DCACHE_LINESIZE) mutable mysql_mutex_t mutex;
-  alignas(CPU_LEVEL1_DCACHE_LINESIZE) ilist<trx_t> trx_list;
-};
-
 /** The transaction system central memory data structure. */
 class trx_sys_t
 {
@@ -858,9 +806,10 @@ class trx_sys_t
   bool m_initialised;
 
 public:
-  /** List of all transactions. */
-  thread_safe_trx_ilist_t trx_list;
+  /** List of all transactions; protected by lock_sys.latch */
+  ilist<trx_t> trx_list;
 
+  alignas(CPU_LEVEL1_DCACHE_LINESIZE)
   /** Temporary rollback segments */
   trx_rseg_t temp_rsegs[TRX_SYS_N_RSEGS];
 
@@ -1102,7 +1051,7 @@ class trx_sys_t
   void close();
 
   /** @return total number of active (non-prepared) transactions */
-  size_t any_active_transactions(size_t *prepared= nullptr);
+  size_t any_active_transactions(size_t *prepared= nullptr) noexcept;
 
 
   /**
@@ -1179,21 +1128,15 @@ class trx_sys_t
 
     @param trx transaction
   */
-  void register_trx(trx_t *trx)
-  {
-    trx_list.push_front(*trx);
-  }
+  inline void register_trx(trx_t *trx) noexcept;
 
 
   /**
     Deregisters transaction in trx_sys.
 
     @param trx transaction
   */
-  void deregister_trx(trx_t *trx)
-  {
-    trx_list.remove(*trx);
-  }
+  inline void deregister_trx(trx_t *trx) noexcept;
 
 
   /**
@@ -1207,17 +1150,7 @@ class trx_sys_t
 
 
   /** @return the number of active views */
-  size_t view_count() const
-  {
-    size_t count= 0;
-
-    trx_list.for_each([&count](const trx_t &trx) {
-      if (trx.read_view.is_open())
-        ++count;
-    });
-
-    return count;
-  }
+  size_t view_count() const noexcept;
 
   /** Disable further allocation of transactions in a rollback segment
   that are subject to innodb_undo_log_truncate=ON

storage/innobase/include/trx0trx.h

@@ -73,10 +73,6 @@ trx_t *trx_create();
 /** At shutdown, frees a transaction object. */
 void trx_free_at_shutdown(trx_t *trx);
 
-/** Disconnect a prepared transaction from MySQL.
-@param[in,out]	trx	transaction */
-void trx_disconnect_prepared(trx_t *trx);
-
 /** Initialize (resurrect) transactions at startup. */
 dberr_t trx_lists_init_at_db_start();
 
@@ -730,7 +726,7 @@ struct trx_t : ilist_node<>
 	This field is accessed by the thread that owns the transaction,
 	without holding any mutex.
 	There is only one foreign-thread access in trx_print_low()
-	and a possible race condition with trx_disconnect_prepared(). */
+	and a possible race condition with disconnect_prepared(). */
 	bool		is_recovered;
 	const char*	op_info;	/*!< English text describing the
 					current operation, or an empty
@@ -968,6 +964,8 @@ struct trx_t : ilist_node<>
 public:
   /** Commit the transaction. */
   void commit() noexcept;
+  /** Disconnect a prepared transaction */
+  void disconnect_prepared() noexcept;
 
   /** Try to drop a persistent table.
   @param table       persistent table

storage/innobase/lock/lock0lock.cc

@@ -5239,43 +5239,30 @@ lock_trx_print_locks(
 	}
 }
 
-/** Functor to display all transactions */
-struct lock_print_info
+/*********************************************************************//**
+Prints info of locks for each transaction. This function will release
+lock_sys.latch, which the caller must be holding in exclusive mode.
+@param file  output stream */
+void lock_print_info_all_transactions(FILE *file)
 {
-  lock_print_info(FILE* file, my_hrtime_t now) :
-    file(file), now(now),
-    purge_trx(purge_sys.query ? purge_sys.query->trx : nullptr)
-  {}
+  fprintf(file, "LIST OF TRANSACTIONS FOR EACH SESSION:\n");
 
-  void operator()(const trx_t &trx) const
+  const trx_t *const purge_trx= purge_sys.query
+    ? purge_sys.query->trx : nullptr;
+  const my_hrtime_t now{my_hrtime_coarse()};
+
+  for (const trx_t &trx : trx_sys.trx_list)
   {
     if (UNIV_UNLIKELY(&trx == purge_trx))
-      return;
+      continue;
     lock_trx_print_wait_and_mvcc_state(file, &trx, now);
 
     if (trx.will_lock && srv_print_innodb_lock_monitor)
       lock_trx_print_locks(file, &trx);
   }
 
-  FILE* const file;
-  const my_hrtime_t now;
-  const trx_t* const purge_trx;
-};
-
-/*********************************************************************//**
-Prints info of locks for each transaction. This function will release
-lock_sys.latch, which the caller must be holding in exclusive mode. */
-void
-lock_print_info_all_transactions(
-/*=============================*/
-	FILE*		file)	/*!< in/out: file where to print */
-{
-	fprintf(file, "LIST OF TRANSACTIONS FOR EACH SESSION:\n");
-
-	trx_sys.trx_list.for_each(lock_print_info(file, my_hrtime_coarse()));
-	lock_sys.wr_unlock();
-
-	ut_d(lock_validate());
+  lock_sys.wr_unlock();
+  ut_d(lock_validate());
 }
 
 #ifdef UNIV_DEBUG

storage/innobase/read/read0read.cc

@@ -29,6 +29,7 @@ Created 2/16/1997 Heikki Tuuri
 #include "srv0srv.h"
 #include "trx0sys.h"
 #include "trx0purge.h"
+#include "lock0lock.h"
 
 /*
 -------------------------------------------------------------------------------
@@ -259,7 +260,8 @@ void trx_sys_t::clone_oldest_view(ReadViewBase *view) const
 {
   view->snapshot(nullptr);
   /* Find oldest view. */
-  trx_list.for_each([view](const trx_t &trx) {
-                      trx.read_view.append_to(view);
-		    });
+  lock_sys.rd_lock(SRW_LOCK_CALL);
+  for (const trx_t &trx : trx_list)
+    trx.read_view.append_to(view);
+  lock_sys.rd_unlock();
 }

storage/innobase/srv/srv0srv.cc

@@ -695,6 +695,20 @@ static void srv_refresh_innodb_monitor_stats(time_t current_time)
 	mysql_mutex_unlock(&srv_innodb_monitor_mutex);
 }
 
+size_t trx_sys_t::view_count() const noexcept
+{
+  ut_ad(lock_sys.is_holder());
+
+  size_t count{0};
+
+  for (const trx_t &trx : trx_list)
+    if (trx.read_view.is_open())
+      ++count;
+
+  return count;
+}
+
+
 /******************************************************************//**
 Outputs to a file the output of the InnoDB Monitor.
 @return FALSE if not all information printed
@@ -760,6 +774,7 @@ srv_printf_innodb_monitor(
 		ut_copy_file(file, dict_foreign_err_file);
 	}
 
+	size_t view_count{0};
 	mysql_mutex_unlock(&dict_foreign_err_mutex);
 
 	/* Only if lock_print_info_summary proceeds correctly,
@@ -778,6 +793,8 @@ srv_printf_innodb_monitor(
 			}
 		}
 
+		view_count = trx_sys.view_count();
+
 		/* NOTE: The following function will release the lock_sys.latch
 		that lock_print_info_summary() acquired. */
 
@@ -848,11 +865,11 @@ srv_printf_innodb_monitor(
 
 	buf_print_io(file);
 
-	fputs("--------------\n"
-	      "ROW OPERATIONS\n"
-	      "--------------\n", file);
-	fprintf(file, ULINTPF " read views open inside InnoDB\n",
-		trx_sys.view_count());
+	fprintf(file,
+		"--------------\n"
+		"ROW OPERATIONS\n"
+		"--------------\n"
+		"%zu read views open inside InnoDB\n", view_count);
 
 	if (ulint n_reserved = fil_system.sys_space->n_reserved_extents) {
 		fprintf(file,