Bug #34857411 : regression - slow connections/telnet block many other statements

                for connect_timeout seconds, causing pileups

Description:
------------
This is a regression caused due to the fix made for the Bug 34094706. When a
connection somehow stalls/blocks during the authentication phase, where a mutex
is held, the other connections that are executing queries on I_S and P_S are
blocked until the first connection release the mutex.

Fix:
----
Instead of using the mutex and checking the thd->active_vio, we now check the
value of net.vio type in the is_secure_transport() check.

Change-Id: Ic301a3a69b2bb828f2323441bdd0232e5a0ecef4

Author: TharunSai

sql/auth/sql_authentication.cc

@@ -2196,14 +2196,6 @@ acl_authenticate(THD *thd, enum_server_command command)
   compile_time_assert(MYSQL_USERNAME_LENGTH == USERNAME_LENGTH);
   assert(command == COM_CONNECT || command == COM_CHANGE_USER);
 
-  mysql_mutex_lock(&thd->LOCK_thd_data);
-
-  DBUG_EXECUTE_IF("before_server_mpvio_initialize", {
-    DBUG_SET("+d,wait_until_thread_kill");
-    const char act[] = "now SIGNAL acl_auth_reached";
-    assert(!debug_sync_set_action(current_thd, STRING_WITH_LEN(act)));
-  });
-
   server_mpvio_initialize(thd, &mpvio, &charset_adapter);
   /*
     Clear thd->db as it points to something, that will be freed when
@@ -2232,7 +2224,6 @@ acl_authenticate(THD *thd, enum_server_command command)
     {
       login_failed_error(&mpvio, mpvio.auth_info.password_used);
       server_mpvio_update_thd(thd, &mpvio);
-      mysql_mutex_unlock(&thd->LOCK_thd_data);
       DBUG_RETURN(1);
     }
 
@@ -2275,19 +2266,11 @@ acl_authenticate(THD *thd, enum_server_command command)
     }
   }
 
-  DBUG_EXECUTE_IF("before_server_mpvio_update_thd", {
-    DBUG_SET("+d,wait_until_thread_kill");
-    const char act[] = "now SIGNAL acl_auth_reached";
-    assert(!debug_sync_set_action(current_thd, STRING_WITH_LEN(act)));
-  });
-
   server_mpvio_update_thd(thd, &mpvio);
 #ifdef HAVE_PSI_THREAD_INTERFACE
   PSI_THREAD_CALL(set_connection_type)(thd->get_vio_type());
 #endif /* HAVE_PSI_THREAD_INTERFACE */
 
-  mysql_mutex_unlock(&thd->LOCK_thd_data);
-
   Security_context *sctx= thd->security_context();
   const ACL_USER *acl_user= mpvio.acl_user;
   bool proxy_check= check_proxy_users && !*mpvio.auth_info.authenticated_as;
@@ -2469,20 +2452,22 @@ acl_authenticate(THD *thd, enum_server_command command)
       DBUG_RETURN(1);
     }
 
-    mysql_mutex_lock(&thd->LOCK_thd_data);
     DBUG_EXECUTE_IF("before_secure_transport_check", {
-      DBUG_SET("+d,wait_until_thread_kill");
-      const char act[] = "now SIGNAL acl_auth_reached";
+      const char act[] = "now SIGNAL kill_now WAIT_FOR killed";
       assert(!debug_sync_set_action(current_thd, STRING_WITH_LEN(act)));
     });
 
-    if (opt_require_secure_transport && thd->active_vio != NULL &&
-        !is_secure_transport(thd->active_vio->type)) {
-      mysql_mutex_unlock(&thd->LOCK_thd_data);
+    /*
+      The assumption here is that thd->active_vio and thd->net.vio are both
+      the same at this point. We should not use thd->active_vio at any cost,
+      as a KILL command can shutdown the active_vio i.e., making it a nullptr
+      which would cause issues. Instead we check the net.vio type.
+    */
+    if (opt_require_secure_transport && thd->get_net()->vio != NULL &&
+        !is_secure_transport(thd->get_net()->vio->type)) {
       my_error(ER_SECURE_TRANSPORT_REQUIRED, MYF(0));
       DBUG_RETURN(1);
     }
-    mysql_mutex_unlock(&thd->LOCK_thd_data);
 
     /* checking password_time_expire for connecting user */
     password_time_expired= check_password_lifetime(thd, mpvio.acl_user);
@@ -2567,11 +2552,6 @@ acl_authenticate(THD *thd, enum_server_command command)
 #endif // !EMBEDDED_LIBRARY
   }
 
-  DBUG_EXECUTE_IF("wait_until_thread_kill", {
-    DBUG_SET("-d,wait_until_thread_kill");
-    const char act[] = "now WAIT_FOR killed";
-    assert(!debug_sync_set_action(current_thd, STRING_WITH_LEN(act)));
-  });
   /*
     This is the default access rights for the current database.  It's
     set to 0 here because we don't have an active database yet (and we

sql/sql_class.h

@@ -2214,6 +2214,8 @@ class THD :public MDL_context_owner,
   NET     net;                          // client connection descriptor
   String  packet;                       // dynamic buffer for network I/O
 public:
+  const NET *get_net() const { return &net; }
+
   void set_skip_readonly_check()
   {
     skip_readonly_check= true;

sql/sql_parse.cc

@@ -1936,7 +1936,6 @@ bool dispatch_command(THD *thd, const COM_DATA *com_data,
   thd->proc_info= 0;
   thd->lex->sql_command= SQLCOM_END;
 
-  DEBUG_SYNC(thd, "processlist_wait");
   /* Performance Schema Interface instrumentation, end */
   MYSQL_END_STATEMENT(thd->m_statement_psi, thd->get_stmt_da());
   thd->m_statement_psi= NULL;
@@ -6449,6 +6448,7 @@ static uint kill_one_thread(THD *thd, my_thread_id id, bool only_kill_query)
 
   DBUG_ENTER("kill_one_thread");
   DBUG_PRINT("enter", ("id=%u only_kill=%d", id, only_kill_query));
+  DEBUG_SYNC(thd, "kill_thd_begin");
   tmp= Global_THD_manager::get_instance()->find_thd(&find_thd_with_id);
   if (tmp)
   {