BUG#31490241 - X.ADMIN_XKILL FAILS WITH VALGRIND ERRORS ON PB2

Description
===========
The issue is a race-condition, seen when a user tries to list
all session, and there is one "Srv-session" in middle of closure.
The "pluggable protocol" is removed from "Srv-session",
and session is left with "text-protocol" instance. Listing such
session causes access to not initialized data, required by
"text-protocol" instance.

Fix
===
At session closure, the patch is going to reverse the deinitialization
sequence. First it is going to remove the `THD` from global container
of all THDs, after that it is going to pop "pluggable protocol" from
protocol stack. The session wont be visible in any session-lists
after removing `THD` from global container, thus its safe to pop
the "pluggable protocol".

RB: 24698
Reviewed by: Grzegorz Szwarc <grzegorz.szwarc@oracle.com>
Reviewed-by: Catalin Besleaga <catalin.besleaga@oracle.com>

Author: lkotula

mysql-test/suite/x/r/session_close.result

@@ -0,0 +1,39 @@
+## I. Reproduce race condition between closing session and listing
+#     session (BUG#31490241):
+# 1. Create X Protocol session(X1) using "x_root" account and hang it while closing
+#    (sync point: srv_session_close)
+# 2. Create classic protocol session(C1) and list all session,
+#    hang it at "x_root" row generation while waking X1
+# 3. Wait until the X1, is inside THD deregistration
+#    and wakeup C1
+# 4. Show at C1, processlist results
+
+# Adding debug point 'test_fill_proc_with_x_root' to @@GLOBAL.debug
+SET DEBUG_SYNC= 'fill_proc_list_with_x_root SIGNAL listing_x_root WAIT_FOR continue_fill_proc';
+
+#
+# I.1 connect X1
+RUN SET DEBUG_SYNC= 'srv_session_close WAIT_FOR listing_x_root'
+
+0 rows affected
+Mysqlx.Ok {
+  msg: "bye!"
+}
+ok
+
+#
+# I.2 Use C1 and list all sessions
+SELECT user from information_schema.processlist order by user;
+
+#
+# I.3
+SET DEBUG_SYNC= 'now SIGNAL continue_fill_proc';
+
+#
+# I.4
+user
+event_scheduler
+root
+root
+x_root
+# Removing debug point 'test_fill_proc_with_x_root' from @@GLOBAL.debug

mysql-test/suite/x/t/session_close.test

@@ -0,0 +1,75 @@
+## This test-case was created for reproducing a Valgrind issue that occure
+#  between two session, one lising all session and second that is closing.
+
+--source include/have_debug_sync.inc
+--source include/have_debug.inc
+--source ../include/have_performance_schema_events_waits_history.inc
+
+--source include/xplugin_preamble.inc
+--source include/xplugin_create_user.inc
+
+--echo ## I. Reproduce race condition between closing session and listing
+--echo #     session (BUG#31490241):
+--echo # 1. Create X Protocol session(X1) using "x_root" account and hang it while closing
+--echo #    (sync point: srv_session_close)
+--echo # 2. Create classic protocol session(C1) and list all session,
+--echo #    hang it at "x_root" row generation while waking X1
+--echo # 3. Wait until the X1, is inside THD deregistration
+--echo #    and wakeup C1
+--echo # 4. Show at C1, processlist results
+--echo
+
+## Test Setup
+--let $xtest_file= $MYSQL_TMP_DIR/admin_ping_mysqlx.tmp
+--write_file $xtest_file
+
+SET DEBUG_SYNC= 'srv_session_close WAIT_FOR listing_x_root';
+EOF
+
+--let $debug_point= test_fill_proc_with_x_root
+--source include/add_debug_point.inc
+
+# Connect C1
+--connect(con1,localhost,root,,)
+SET DEBUG_SYNC= 'fill_proc_list_with_x_root SIGNAL listing_x_root WAIT_FOR continue_fill_proc';
+
+
+## Test execution
+--echo
+--echo #
+--echo # I.1 connect X1
+--exec $MYSQLXTEST -ux_root --password='' --file=$xtest_file 2>&1
+
+--echo
+--echo #
+--echo # I.2 Use C1 and list all sessions
+send SELECT user from information_schema.processlist order by user;
+
+
+--echo
+--echo #
+--echo # I.3
+# Go back to C0, wait for X1
+--connection default
+let $wait_condition=
+  select count(*) from performance_schema.events_waits_current
+       where EVENT_NAME like 'wait/synch/mutex/sql/LOCK_thd_remove';
+--source include/wait_condition_or_abort.inc
+
+# Wake up C1
+SET DEBUG_SYNC= 'now SIGNAL continue_fill_proc';
+
+--echo
+--echo #
+--echo # I.4
+# Go back to C1, and receive process-list
+--connection con1
+reap;
+
+## Cleanup
+--connection default
+
+--let $debug_point= test_fill_proc_with_x_root
+--source include/remove_debug_point.inc
+--remove_file $xtest_file
+--source include/xplugin_drop_user.inc

sql/sql_show.cc

@@ -2283,6 +2283,12 @@ class Fill_process_list : public Do_THD_Impl {
                   strcmp(inspect_sctx_user.str, user))))
       return;
 
+    DBUG_EXECUTE_IF(
+        "test_fill_proc_with_x_root",
+        if (0 == strcmp(inspect_sctx_user.str, "x_root")) {
+          DEBUG_SYNC(m_client_thd, "fill_proc_list_with_x_root");
+        });
+
     TABLE *table = m_tables->table;
     restore_record(table, s->default_values);
 

sql/srv_session.cc

@@ -60,6 +60,7 @@
 #include "sql/auth/sql_security_ctx.h"
 #include "sql/conn_handler/connection_handler_manager.h"
 #include "sql/current_thd.h"
+#include "sql/debug_sync.h"          // DEBUG_SYNC
 #include "sql/derror.h"              // ER_DEFAULT
 #include "sql/log.h"                 // Query log
 #include "sql/mysqld.h"              // current_thd
@@ -1017,6 +1018,13 @@ bool Srv_session::close() {
 
   thd.get_stmt_da()->reset_diagnostics_area();
 
+  // DEBUG_SYNC control block is released under call to
+  // `thd.release_resource`, thus we can't put this sync
+  // point directly before `pop_protocol`.
+  // Second constrain is that `THD::disconnect` marks
+  // this connection as killed, which disables DEBUG_SYNC.
+  DEBUG_SYNC(&thd, "srv_session_close");
+
   thd.disconnect();
 
 #ifdef HAVE_PSI_THREAD_INTERFACE
@@ -1025,12 +1033,12 @@ bool Srv_session::close() {
 
   thd.release_resources();
 
+  Global_THD_manager::get_instance()->remove_thd(&thd);
+
   mysql_mutex_lock(&thd.LOCK_thd_protocol);
   thd.pop_protocol();
   mysql_mutex_unlock(&thd.LOCK_thd_protocol);
 
-  Global_THD_manager::get_instance()->remove_thd(&thd);
-
   Connection_handler_manager::dec_connection_count();
 
   return false;