BUG#26886646 - SEGMENTATION FAULT AT TASK_DELETE() IN LIBMYSQLGCS/SRC/BINDINGS/XCOM/XCOM/TASK.C

Problem
----------------------------

A Segmentation fault was encountered on a daily-trunk pushbuild
run for the test 'group_replication.gr_ssl_mode_verify_identity_error'
when starting group replication with the below symptoms -

group_replication.gr_ssl_mode_verify_identity_error w5 [ fail ]
        Test ended at 2017-09-27 17:37:18

CURRENT_TEST: group_replication.gr_ssl_mode_verify_identity_error
mysqltest: At line 58: query 'START GROUP_REPLICATION' failed with wrong
errno 2013: 'Lost connection to MySQL server during query', instead of
3092

Analysis
-----------------------------

The test that fails sporadically is Step #2 of gr_ssl_mode_verify_identity_error.
That step causes a global SSL comunication failure, in which not even the
local connections can be established, thus causing the join operation to fail.

When that happens, GCS will request the XCom thread termination. Since it does
not have socket access, it will contact it directly calling xcom_fsm(exit),
but this is a well-known non-thread safe operation, hence causing this crash.

Fix
-----------------------------

Create a new callback for GCS called should_exit, which is tested within
task_loop, in order to check if we should exit the XCom thread.

Author: tiagoportelajorge

rapid/plugin/group_replication/libmysqlgcs/src/bindings/xcom/gcs_xcom_control_interface.cc

@@ -84,6 +84,7 @@ static void *xcom_taskmain_startup(void *ptr)
   Gcs_xcom_proxy *proxy= gcs_ctrl->get_xcom_proxy();
   xcom_port port= gcs_ctrl->get_node_address()->get_member_port();
 
+  proxy->set_should_exit(false);
   proxy->xcom_init(port);
 
   My_xp_thread_util::exit(0);

rapid/plugin/group_replication/libmysqlgcs/src/bindings/xcom/gcs_xcom_interface.cc

@@ -76,6 +76,7 @@ void      cb_xcom_comms(int status);
 void      cb_xcom_ready(int status);
 void      cb_xcom_exit(int status);
 synode_no cb_xcom_get_app_snap(blob *gcs_snap);
+int       cb_xcom_get_should_exit();
 void      cb_xcom_handle_app_snap(blob *gcs_snap);
 int       cb_xcom_socket_accept(int fd);
 
@@ -881,6 +882,7 @@ initialize_xcom(const Gcs_interface_parameters &interface_params)
   ::set_xcom_global_view_receiver(cb_xcom_receive_global_view);
   ::set_port_matcher(cb_xcom_match_port);
   ::set_app_snap_handler(cb_xcom_handle_app_snap);
+  ::set_should_exit_getter(cb_xcom_get_should_exit);
   ::set_app_snap_getter(cb_xcom_get_app_snap);
   ::set_xcom_run_cb(cb_xcom_ready);
   ::set_xcom_comms_cb(cb_xcom_comms);
@@ -1579,6 +1581,13 @@ synode_no cb_xcom_get_app_snap(blob *gcs_snap MY_ATTRIBUTE((unused)))
   return null_synode;
 }
 
+int cb_xcom_get_should_exit()
+{
+  if (xcom_proxy)
+    return (int)xcom_proxy->get_should_exit();
+  else
+    return 0;
+}
 
 void cb_xcom_ready(int status MY_ATTRIBUTE((unused)))
 {

rapid/plugin/group_replication/libmysqlgcs/src/bindings/xcom/gcs_xcom_utils.cc

@@ -298,7 +298,8 @@ int Gcs_xcom_proxy_impl::xcom_exit(bool xcom_handlers_open)
   else if (!xcom_handlers_open)
   {
     /* The handlers were not yet open, so use basic xcom stop */
-    ::xcom_fsm(xa_exit, int_arg(0));
+    this->set_should_exit(1);
+
     res= false;
   }
 
@@ -506,7 +507,8 @@ Gcs_xcom_proxy_impl::Gcs_xcom_proxy_impl()
    m_crl_file(),
    m_crl_path(),
    m_cipher(),
-   m_tls_version()
+   m_tls_version(),
+   m_should_exit(false)
 {
   m_xcom_handlers= new Xcom_handler *[m_xcom_handlers_size];
 
@@ -550,7 +552,8 @@ Gcs_xcom_proxy_impl::Gcs_xcom_proxy_impl(unsigned int wt)
    m_crl_file(),
    m_crl_path(),
    m_cipher(),
-   m_tls_version()
+   m_tls_version(),
+   m_should_exit(false)
 {
   m_xcom_handlers= new Xcom_handler *[m_xcom_handlers_size];
 
@@ -866,6 +869,18 @@ Gcs_xcom_proxy_impl::xcom_signal_comms_status_changed(int status)
   m_lock_xcom_comms_status.unlock();
 }
 
+bool
+Gcs_xcom_proxy_impl::get_should_exit()
+{
+  return m_should_exit.load(std::memory_order_relaxed);
+}
+
+void
+Gcs_xcom_proxy_impl::set_should_exit(bool should_exit)
+{
+  m_should_exit.store(should_exit, std::memory_order_relaxed);
+}
+
 void Gcs_xcom_app_cfg::init()
 {
   ::init_cfg_app_xcom();

rapid/plugin/group_replication/libmysqlgcs/src/bindings/xcom/gcs_xcom_utils.h

@@ -16,6 +16,7 @@
 #ifndef GCS_XCOM_UTILS_INCLUDED
 #define GCS_XCOM_UTILS_INCLUDED
 
+#include <atomic>
 #include <string>
 #include <vector>
 
@@ -636,6 +637,20 @@ class Gcs_xcom_proxy
 
   virtual int xcom_force_nodes(Gcs_xcom_nodes &nodes,
                                unsigned int group_id_hash)= 0;
+
+  /**
+    Function that retrieves the value that signals that XCom
+    must be forcefully stopped.
+
+    @return 1 if XCom needs to forcefully exit. 0 otherwise.
+   */
+  virtual bool get_should_exit()= 0;
+
+  /**
+    Function that sets the value that signals that XCom
+    must be forcefully stopped.
+   */
+  virtual void set_should_exit(bool should_exit)= 0;
 };
 
 
@@ -759,6 +774,8 @@ class Gcs_xcom_proxy_impl : public Gcs_xcom_proxy_base
   int xcom_client_force_config(node_list *nl, uint32_t group_id);
   int xcom_client_force_config(connection_descriptor *fd, node_list *nl,
                                uint32_t group_id);
+  bool get_should_exit();
+  void set_should_exit(bool should_exit);
 private:
   /* A pointer to the next local XCom connection to use. */
   int m_xcom_handlers_cursor;
@@ -805,6 +822,7 @@ class Gcs_xcom_proxy_impl : public Gcs_xcom_proxy_base
   const char *m_cipher;
   const char *m_tls_version;
 
+  std::atomic_bool m_should_exit;
 
   /*
     Disabling the copy constructor and assignment operator.

rapid/plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/task.c

@@ -71,6 +71,8 @@
 #include "plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/task_net.h"
 #include "plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/task_os.h"
 #include "plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/xcom_cfg.h"
+#include "plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/site_def.h"
+#include "plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/xcom_base.h"
 
 #ifndef _WIN32
 #include <poll.h>
@@ -935,11 +937,22 @@ static int msdiff(double time) {
   return (int)(1000.5 * (first_delayed()->time - time));
 }
 
+static should_exit_getter get_should_exit;
+
+void set_should_exit_getter(should_exit_getter x)  { get_should_exit = x; }
+
 static double idle_time = 0.0;
 void task_loop() {
+  task_env *t = 0;
   /* While there are tasks */
   for (;;) {
-    task_env *t = first_runnable();
+    //check forced exit callback
+    if(get_should_exit())
+    {
+      xcom_fsm(xa_exit, int_arg(0));
+    }
+
+    t = first_runnable();
     /* While runnable tasks */
     while (runnable_tasks()) {
       task_env *next = next_task(t);
@@ -1023,9 +1036,9 @@ static int init_sockaddr(char *server, struct sockaddr_in *sock_addr,
                          socklen_t *sock_size, xcom_port port) {
   /* Get address of server */
   struct addrinfo *addr = 0;
-  
+
   checked_getaddrinfo(server, 0, 0, &addr);
-  
+
   if (!addr) return 0;
   /* Copy first address */
   memcpy(sock_addr, addr->ai_addr, addr->ai_addrlen);

rapid/plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/task.h

@@ -26,6 +26,7 @@
 #include "plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/x_platform.h"
 #include "plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/xcom_common.h"
 
+
 #ifdef __cplusplus
 extern "C" {
 #endif

rapid/plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/xcom_base.h

@@ -27,7 +27,6 @@ extern "C" {
 #include "plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/xcom_os_layer.h"
 #include "plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/xcom_os_layer.h"
 #include "plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/xdr_utils.h"
-#include "plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/xdr_utils.h"
 
 #define XCOM_THREAD_DEBUG 1
 
@@ -183,6 +182,9 @@ void set_xcom_run_cb(xcom_state_change_cb x);
 void set_xcom_terminate_cb(xcom_state_change_cb x);
 void set_xcom_exit_cb(xcom_state_change_cb x);
 
+typedef int (*should_exit_getter)();
+void set_should_exit_getter(should_exit_getter x);
+
 app_data_ptr init_config_with_group(app_data *a, node_list *nl, cargo_type type,
                                     uint32_t group_id);
 

rapid/unittest/gunit/libmysqlgcs/xcom/gcs_xcom_communication_interface-t.cc

@@ -126,6 +126,9 @@ class mock_gcs_xcom_proxy : public Gcs_xcom_proxy_base
                                              uint32_t group_id));
   MOCK_METHOD2(xcom_client_force_config, int(node_list *nl,
                                              uint32_t group_id));
+
+  MOCK_METHOD0(get_should_exit, bool());
+  MOCK_METHOD1(set_should_exit,void(bool should_exit));
 };
 
 

rapid/unittest/gunit/libmysqlgcs/xcom/gcs_xcom_group_management-t.cc

@@ -91,6 +91,9 @@ class mock_gcs_xcom_proxy : public Gcs_xcom_proxy_base
                                              uint32_t group_id));
   MOCK_METHOD2(xcom_client_force_config, int(node_list *nl,
                                              uint32_t group_id));
+
+  MOCK_METHOD0(get_should_exit, bool());
+  MOCK_METHOD1(set_should_exit,void(bool should_exit));
 };
 
 class XcomGroupManagementTest : public GcsBaseTest