BUG#36093405: Signal 11 seen in Gtid_set::~Gtid_set

Group Replication maintains a memory structure that keeps track of
transactions accepted to commit but not committed on all members
yet. This structure, named certification info, is used to detect
conflicts and dependencies between transactions.
The certification info is cleaned periodically and on Group
Replication stop.
There was a race identified between these two operations, more
precisely:
 1) Certifier::garbage_collect()
    -> while (it != certification_info.end()) {
         if (it->second->is_subset_not_equals(stable_gtid_set)) {
           if (it->second->unlink() == 0) delete it->second;
 2) Certifier::~Certifier()
    -> clear_certification_info();
       -> for (Certification_info::iterator it = certification_info.begin();
              it != certification_info.end(); ++it) {
            if (it->second->unlink() == 0) delete it->second;

`clear_certification_info()` was being called without securing
exclusive access to `certification_info` which could cause
concurrent access to its items, more precisely `delete it->second`.

To solve the above issue, `~Certifier()` (like all other callers) do
secure the exclusive access to certification info.

Change-Id: I28111d41adb54248d90137ee9d2c17196de045e8

Author: nacarvalho

plugin/group_replication/include/certifier.h

@@ -25,6 +25,7 @@
 
 #include <assert.h>
 #include <mysql/group_replication_priv.h>
+#include <atomic>
 #include <list>
 #include <map>
 #include <string>
@@ -423,7 +424,7 @@ class Certifier : public Certifier_interface {
   /**
     Is certifier initialized.
   */
-  bool initialized;
+  std::atomic<bool> initialized{false};
 
   /**
     Variable to store the sidno used for transactions which will be logged

plugin/group_replication/src/certifier.cc

@@ -300,16 +300,19 @@ Certifier::Certifier()
 }
 
 Certifier::~Certifier() {
+  mysql_mutex_lock(&LOCK_certification_info);
+  initialized = false;
   clear_certification_info();
   delete certification_info_sid_map;
 
   delete stable_gtid_set;
   delete stable_sid_map;
   delete stable_gtid_set_lock;
-  delete broadcast_thread;
   delete group_gtid_executed;
   delete group_gtid_extracted;
   delete group_gtid_sid_map;
+  mysql_mutex_unlock(&LOCK_certification_info);
+  delete broadcast_thread;
 
   mysql_mutex_lock(&LOCK_members);
   clear_members();
@@ -566,6 +569,7 @@ void Certifier::add_to_group_gtid_executed_internal(rpl_sidno sidno,
 }
 
 void Certifier::clear_certification_info() {
+  mysql_mutex_assert_owner(&LOCK_certification_info);
   for (Certification_info::iterator it = certification_info.begin();
        it != certification_info.end(); ++it) {
     // We can only delete the last reference.
@@ -926,6 +930,10 @@ rpl_gno Certifier::certify(Gtid_set *snapshot_version,
 int Certifier::add_specified_gtid_to_group_gtid_executed(Gtid_log_event *gle) {
   DBUG_TRACE;
 
+  if (!is_initialized()) {
+    return 1;
+  }
+
   mysql_mutex_lock(&LOCK_certification_info);
   rpl_sidno sidno = gle->get_sidno(group_gtid_sid_map);
 
@@ -951,6 +959,11 @@ int Certifier::add_specified_gtid_to_group_gtid_executed(Gtid_log_event *gle) {
 
 int Certifier::add_group_gtid_to_group_gtid_executed(rpl_gno gno) {
   DBUG_TRACE;
+
+  if (!is_initialized()) {
+    return 1;
+  }
+
   mysql_mutex_lock(&LOCK_certification_info);
   add_to_group_gtid_executed_internal(group_gtid_sid_map_group_sidno, gno);
   mysql_mutex_unlock(&LOCK_certification_info);
@@ -1159,6 +1172,10 @@ int Certifier::get_group_stable_transactions_set_string(char **buffer,
   DBUG_TRACE;
   int error = 1;
 
+  if (!is_initialized()) {
+    return 1;
+  }
+
   /*
     Stable transactions set may not be accurate during recovery,
     thence we do not externalize it on
@@ -1208,6 +1225,11 @@ bool Certifier::set_group_stable_transactions_set(Gtid_set *executed_gtid_set) {
 
 void Certifier::garbage_collect() {
   DBUG_TRACE;
+
+  if (!is_initialized()) {
+    return;
+  }
+
   /*
     This debug option works together with
     `group_replication_certifier_broadcast_thread_big_period`
@@ -1484,6 +1506,11 @@ int Certifier::stable_set_handle() {
 
 void Certifier::handle_view_change() {
   DBUG_TRACE;
+
+  if (!is_initialized()) {
+    return;
+  }
+
   mysql_mutex_lock(&LOCK_members);
   clear_incoming();
   clear_members();
@@ -1493,6 +1520,11 @@ void Certifier::handle_view_change() {
 void Certifier::get_certification_info(
     std::map<std::string, std::string> *cert_info) {
   DBUG_TRACE;
+
+  if (!is_initialized()) {
+    return;
+  }
+
   mysql_mutex_lock(&LOCK_certification_info);
 
   for (Certification_info::iterator it = certification_info.begin();
@@ -1524,6 +1556,10 @@ void Certifier::get_certification_info(
 Gtid Certifier::generate_view_change_group_gtid() {
   DBUG_TRACE;
 
+  if (!is_initialized()) {
+    return {-1, -1};
+  }
+
   mysql_mutex_lock(&LOCK_certification_info);
   rpl_gno result =
       get_next_available_gtid(nullptr, views_sidno_group_representation);
@@ -1544,6 +1580,10 @@ int Certifier::set_certification_info(
   DBUG_TRACE;
   assert(cert_info != nullptr);
 
+  if (!is_initialized()) {
+    return 1;
+  }
+
   if (cert_info->size() == 1) {
     std::map<std::string, std::string>::iterator it =
         cert_info->find(CERTIFICATION_INFO_ERROR_NAME);
@@ -1674,6 +1714,10 @@ void Certifier::get_last_conflict_free_transaction(std::string *value) {
   int length = 0;
   char buffer[Gtid::MAX_TEXT_LENGTH + 1];
 
+  if (!is_initialized()) {
+    return;
+  }
+
   mysql_mutex_lock(&LOCK_certification_info);
   if (last_conflict_free_transaction.is_empty()) goto end;
 
@@ -1687,6 +1731,10 @@ void Certifier::get_last_conflict_free_transaction(std::string *value) {
 void Certifier::enable_conflict_detection() {
   DBUG_TRACE;
 
+  if (!is_initialized()) {
+    return;
+  }
+
   mysql_mutex_lock(&LOCK_certification_info);
   conflict_detection_enable = true;
   local_member_info->enable_conflict_detection();
@@ -1697,6 +1745,10 @@ void Certifier::disable_conflict_detection() {
   DBUG_TRACE;
   assert(local_member_info->in_primary_mode());
 
+  if (!is_initialized()) {
+    return;
+  }
+
   mysql_mutex_lock(&LOCK_certification_info);
   conflict_detection_enable = false;
   local_member_info->disable_conflict_detection();
@@ -1708,6 +1760,10 @@ void Certifier::disable_conflict_detection() {
 bool Certifier::is_conflict_detection_enable() {
   DBUG_TRACE;
 
+  if (!is_initialized()) {
+    return false;
+  }
+
   mysql_mutex_lock(&LOCK_certification_info);
   bool result = conflict_detection_enable;
   mysql_mutex_unlock(&LOCK_certification_info);