MDEV-24114 : Crash: WSREP: invalid state ROLLED_BACK (FATAL)10.2-MDEV-25114

Reverts fix for MDEV-23328 i.e. commit 29bbcac0ee841

In this fix we try to modify normal SQL KILL in a following way:

trx_id= thd_to_trx(victim_thd)->id;
mutex_unlock(victim_thd->LOCK_thd_data);
mutex_unlock(victim_thd->LOCK_thd_kill);
lock_mutex_enter
trx=find_and_lock_trx_by_id(trx_id)
mytex_lock(trx->mysql_thd->LOCK_thd_kill);
mutex_lock(trx->mysql_thd->LOCK_thd_data)

For THD::awake() we use:

mutex_lock(thd->LOCK_thd_kill);
mutex_lock(thd->LOCK_thd_data);
thd->awake();
mutex_unlock(thd->LOCK_thd_data);
mutex_unlock(thd->LOCK_thd_kill);

For THD::set_killed in most cases we use

mutex_lock(thd->LOCK_thd_kill);
mutex_lock(thd->LOCK_thd_data);
thd->set_killed_no_mutex(...);
mutex_unlock(thd->LOCK_thd_data);
mutex_unlock(thd->LOCK_thd_kill);

1 files changed, 26 insertions, 13 deletions

diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 5ada018e540..c72d192efa5 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -593,10 +593,12 @@ handle_condition(THD *thd,
 extern "C" void thd_kill_timeout(THD* thd)
 {
   thd->status_var.max_statement_time_exceeded++;
+  mysql_mutex_lock(&thd->LOCK_thd_kill);
   mysql_mutex_lock(&thd->LOCK_thd_data);
   /* Kill queries that can't cause data corruptions */
   thd->awake(KILL_TIMEOUT);
   mysql_mutex_unlock(&thd->LOCK_thd_data);
+  mysql_mutex_unlock(&thd->LOCK_thd_kill);
 }
 
 
@@ -1693,6 +1695,7 @@ void THD::awake(killed_state state_to_set)
   DBUG_PRINT("enter", ("this: %p current_thd: %p  state: %d",
                        this, current_thd, (int) state_to_set));
   THD_CHECK_SENTRY(this);
+  mysql_mutex_assert_owner(&LOCK_thd_kill);
   mysql_mutex_assert_owner(&LOCK_thd_data);
 
   print_aborted_warning(3, "KILLED");
@@ -1705,7 +1708,6 @@ void THD::awake(killed_state state_to_set)
     state_to_set= killed;
 
   /* Set the 'killed' flag of 'this', which is the target THD object. */
-  mysql_mutex_lock(&LOCK_thd_kill);
   set_killed_no_mutex(state_to_set);
 
   if (state_to_set >= KILL_CONNECTION || state_to_set == NOT_KILLED)
@@ -1792,7 +1794,6 @@ void THD::awake(killed_state state_to_set)
     }
     mysql_mutex_unlock(&mysys_var->mutex);
   }
-  mysql_mutex_unlock(&LOCK_thd_kill);
   DBUG_VOID_RETURN;
 }
 
@@ -1808,9 +1809,10 @@ void THD::disconnect()
 {
   Vio *vio= NULL;
 
+  mysql_mutex_lock(&LOCK_thd_kill);
   mysql_mutex_lock(&LOCK_thd_data);
 
-  set_killed(KILL_CONNECTION);
+  set_killed_no_mutex(KILL_CONNECTION);
 
 #ifdef SIGNAL_WITH_VIO_CLOSE
   /*
@@ -1828,6 +1830,7 @@ void THD::disconnect()
   net.thd= 0;                                   // Don't collect statistics
 
   mysql_mutex_unlock(&LOCK_thd_data);
+  mysql_mutex_unlock(&LOCK_thd_kill);
 }
 
 
@@ -1844,9 +1847,10 @@ bool THD::notify_shared_lock(MDL_context_owner *ctx_in_use,
   {
     /* This code is similar to kill_delayed_threads() */
     DBUG_PRINT("info", ("kill delayed thread"));
+    mysql_mutex_lock(&in_use->LOCK_thd_kill);
     mysql_mutex_lock(&in_use->LOCK_thd_data);
     if (in_use->killed < KILL_CONNECTION)
-      in_use->set_killed(KILL_CONNECTION);
+      in_use->set_killed_no_mutex(KILL_CONNECTION);
     if (in_use->mysys_var)
     {
       mysql_mutex_lock(&in_use->mysys_var->mutex);
@@ -1858,11 +1862,14 @@ bool THD::notify_shared_lock(MDL_context_owner *ctx_in_use,
       mysql_mutex_unlock(&in_use->mysys_var->mutex);
     }
     mysql_mutex_unlock(&in_use->LOCK_thd_data);
+    mysql_mutex_unlock(&in_use->LOCK_thd_kill);
     signalled= TRUE;
   }
 
   if (needs_thr_lock_abort)
   {
+    bool need_mutex_release= true;
+    mysql_mutex_lock(&in_use->LOCK_thd_kill);
     mysql_mutex_lock(&in_use->LOCK_thd_data);
     /* If not already dying */
     if (in_use->killed != KILL_CONNECTION_HARD)
@@ -1879,18 +1886,24 @@ bool THD::notify_shared_lock(MDL_context_owner *ctx_in_use,
           thread can see those instances (e.g. see partitioning code).
         */
         if (!thd_table->needs_reopen())
-        {
           signalled|= mysql_lock_abort_for_thread(this, thd_table);
-          if (WSREP(this) && wsrep_thd_is_BF(this, FALSE))
-          {
-            WSREP_DEBUG("remove_table_from_cache: %llu",
-                        (unsigned long long) this->real_id);
-            wsrep_abort_thd((void *)this, (void *)in_use, FALSE);
-          }
-        }
       }
+
+#ifdef WITH_WSREP
+      if (WSREP(this) && wsrep_thd_is_BF(this, FALSE))
+      {
+        WSREP_DEBUG("remove_table_from_cache: %llu",
+                    (unsigned long long) this->real_id);
+        wsrep_abort_thd((void *)this, (void *)in_use, FALSE);
+	need_mutex_release= false;
+      }
+#endif /* WITH_WSREP */
+    }
+    if (need_mutex_release)
+    {
+      mysql_mutex_unlock(&in_use->LOCK_thd_data);
+      mysql_mutex_unlock(&in_use->LOCK_thd_kill);
     }
-    mysql_mutex_unlock(&in_use->LOCK_thd_data);
   }
   DBUG_RETURN(signalled);
 }