Bug#27150732: DOUBLE FREE AT SQL_SERVICE_COMMAND_INTERFACE CLASS

Problem: During SQL Thread error (before thread set running variable), spawned
SQL thread may block waiting for run while launcher thread will do the cleanup
because of error.

--------------------------------------------------------------------------------

Launcher thread has this condition:
  while (m_session_thread_running.to_run() && !m_session_thread_error)

POINT 1: m_session_thread_error, during error it will not block.

--------------------------------------------------------------------------------

Session_plugin_thread::terminate_session_thread is critical.
..  while (m_session_thread_running)

POINT 2:
Session_plugin_thread::terminate_session_thread is critical, since its makes
sure spawned thread is terminated.
But terminate_session_thread got tricked because running variable was never set
to TRUE.
Spawned thread never got mutex to set running=true.

--------------------------------------------------------------------------------

Resolution:
There is missing *_starting(e.g. recovery has recovery_starting) variable in
SQL Service Thread. This variables makes sure spawned thread is terminated
even if it never ran.

Author: Jaideep Karande

rapid/plugin/group_replication/include/sql_service/sql_service_command.h

@@ -210,6 +210,8 @@ class Session_plugin_thread
   long m_method_execution_return_value;
   /** Session thread running flag */
   bool m_session_thread_running;
+  /** Session thread starting flag */
+  bool m_session_thread_starting;
   /** Session termination flag */
   bool m_session_thread_terminate;
   /** Session tread error flag */

rapid/plugin/group_replication/src/sql_service/sql_service_command.cc

@@ -62,7 +62,8 @@ establish_session_connection(enum_plugin_con_isolation isolation_param,
     case PSESSION_DEDICATED_THREAD:
       m_plugin_session_thread = new Session_plugin_thread(&sql_service_commands);
       error = m_plugin_session_thread->launch_session_thread(plugin_pointer);
-      m_server_interface = m_plugin_session_thread->get_service_interface();
+      if (!error)
+        m_server_interface = m_plugin_session_thread->get_service_interface();
       break;
   }
 
@@ -486,7 +487,8 @@ Session_plugin_thread(Sql_service_commands* command_interface)
   : command_interface(command_interface), m_server_interface(NULL),
    incoming_methods(NULL), m_plugin_pointer(NULL),
    m_method_execution_completed(false), m_method_execution_return_value(0),
-   m_session_thread_running(false), m_session_thread_terminate(false),
+   m_session_thread_running(false), m_session_thread_starting(false),
+   m_session_thread_terminate(false),
    m_session_thread_error(0)
 {
   mysql_mutex_init(key_GR_LOCK_session_thread_run, &m_run_lock,
@@ -551,6 +553,7 @@ Session_plugin_thread::launch_session_thread(void* plugin_pointer_var)
 
   m_session_thread_error= 0;
   m_session_thread_terminate= false;
+  m_session_thread_starting= true;
   m_plugin_pointer= plugin_pointer_var;
 
   if ((mysql_thread_create(key_GR_THD_plugin_session,
@@ -559,6 +562,7 @@ Session_plugin_thread::launch_session_thread(void* plugin_pointer_var)
                            launch_handler_thread,
                            (void*)this)))
   {
+    m_session_thread_starting= false;
     mysql_mutex_unlock(&m_run_lock); /* purecov: inspected */
     DBUG_RETURN(1);                /* purecov: inspected */
   }
@@ -585,7 +589,7 @@ Session_plugin_thread::terminate_session_thread()
 
   int stop_wait_timeout= GR_PLUGIN_SESSION_THREAD_TIMEOUT;
 
-  while (m_session_thread_running)
+  while (m_session_thread_running || m_session_thread_starting)
   {
     DBUG_PRINT("loop", ("killing plugin session thread"));
 
@@ -601,7 +605,7 @@ Session_plugin_thread::terminate_session_thread()
     {
       stop_wait_timeout= stop_wait_timeout - 1;
     }
-    else if (m_session_thread_running) // quit waiting
+    else if (m_session_thread_running || m_session_thread_starting) // quit waiting
     {
       mysql_mutex_unlock(&m_run_lock);
       DBUG_RETURN(1);
@@ -632,8 +636,11 @@ Session_plugin_thread::session_thread_handler()
   m_server_interface= new Sql_service_interface();
   m_session_thread_error=
     m_server_interface->open_thread_session(m_plugin_pointer);
+  DBUG_EXECUTE_IF("group_replication_sql_service_force_error",
+                  { m_session_thread_error= 1; });
 
   mysql_mutex_lock(&m_run_lock);
+  m_session_thread_starting= false;
   m_session_thread_running= true;
   mysql_cond_broadcast(&m_run_cond);
   mysql_mutex_unlock(&m_run_lock);

rapid/plugin/group_replication/tests/mtr/r/gr_sql_service_failure.result

@@ -0,0 +1,29 @@
+include/group_replication.inc
+Warnings:
+Note	####	Sending passwords in plain text without SSL/TLS is extremely insecure.
+Note	####	Storing MySQL user name or password information in the master info repository is not secure and is therefore not recommended. Please consider using the USER and PASSWORD connection options for START SLAVE; see the 'START SLAVE Syntax' in the MySQL Manual for more information.
+[connection server1]
+SET SESSION sql_log_bin= 0;
+call mtr.add_suppression("Can't establish a internal server connection to execute plugin operations.*");
+call mtr.add_suppression("Error calling group communication interfaces while trying to leave the group");
+SET SESSION sql_log_bin= 1;
+
+# 1. Start GR. Force SQL Service to return error failing Start GR.
+
+SET @debug_save= @@GLOBAL.DEBUG;
+SET @@GLOBAL.DEBUG= '+d,group_replication_sql_service_force_error';
+START GROUP_REPLICATION;
+ERROR HY000: The server is not configured properly to be an active member of the group. Please see more details on error log.
+
+# 2. Start GR successfully now.
+
+SET @@GLOBAL.DEBUG= '-d,group_replication_sql_service_force_error';
+include/start_and_bootstrap_group_replication.inc
+[connection server2]
+include/start_group_replication.inc
+
+# 3. Clean up.
+
+[connection server1]
+SET @@GLOBAL.DEBUG= @debug_save;
+include/group_replication_end.inc

rapid/plugin/group_replication/tests/mtr/t/gr_sql_service_failure.test

@@ -0,0 +1,56 @@
+################################################################################
+# Validate that Group Replication behaves properly when SQL service does fail.
+#
+# Test:
+# 0. The test requires one servers: M1.
+# 1. Start GR. Force SQL Service to return error failing Start GR.
+# 2. Start GR successfully now.
+# 3. Clean up.
+################################################################################
+--source include/have_debug.inc
+--source ../inc/have_group_replication_plugin.inc
+--let $rpl_skip_group_replication_start= 1
+--source ../inc/group_replication.inc
+
+SET SESSION sql_log_bin= 0;
+call mtr.add_suppression("Can't establish a internal server connection to execute plugin operations.*");
+call mtr.add_suppression("Error calling group communication interfaces while trying to leave the group");
+SET SESSION sql_log_bin= 1;
+
+--echo
+--echo # 1. Start GR. Force SQL Service to return error failing Start GR.
+--echo
+
+SET @debug_save= @@GLOBAL.DEBUG;
+SET @@GLOBAL.DEBUG= '+d,group_replication_sql_service_force_error';
+
+--disable_query_log
+SET GLOBAL group_replication_bootstrap_group=ON;
+--eval SET GLOBAL group_replication_group_name= "$group_replication_group_name"
+--enable_query_log
+
+--error ER_GROUP_REPLICATION_CONFIGURATION
+START GROUP_REPLICATION;
+--source ../inc/gr_clear_bootstrap_group.inc
+
+--echo
+--echo # 2. Start GR successfully now.
+--echo
+
+SET @@GLOBAL.DEBUG= '-d,group_replication_sql_service_force_error';
+--source ../inc/start_and_bootstrap_group_replication.inc
+
+--let $rpl_connection_name= server2
+--source include/rpl_connection.inc
+
+--source include/start_group_replication.inc
+
+--echo
+--echo # 3. Clean up.
+--echo
+
+--let $rpl_connection_name= server1
+--source include/rpl_connection.inc
+
+SET @@GLOBAL.DEBUG= @debug_save;
+--source ../inc/group_replication_end.inc