MDEV-3984: Double free of Master_info * when CHANGE MASTER fails.

When CHANGE MASTER fails, it may or may not have already added
the Master_info * to the index. Implement logic that properly
handles removal and freeing in both cases.

mysql-test/suite/multi_source/multisource.result

@@ -1,3 +1,7 @@
+change master 'abc' to relay_log_file='';
+ERROR HY000: Failed initializing relay log position: Could not find target log during relay log initialization
+change master 'abc2' to master_host='';
+ERROR HY000: Incorrect arguments to MASTER_HOST
 change master 'master1' to 
 master_port=MYPORT_1, 
 master_host='127.0.0.1', 

mysql-test/suite/multi_source/multisource.test

@@ -8,6 +8,15 @@
 
 --connect (slave,127.0.0.1,root,,,$SERVER_MYPORT_3)
 
+# MDEV-3984: crash/read of freed memory when changing master with named connection
+# This fails after adding the new master 'abc', check we do not free twice.
+--error ER_RELAY_LOG_INIT
+change master 'abc' to relay_log_file='';
+# This fails before adding the new master, check that we do free it.
+--error ER_WRONG_ARGUMENTS
+change master 'abc2' to master_host='';
+
+
 # Start replication from the first master
 
 --replace_result $SERVER_MYPORT_1 MYPORT_1

sql/sql_parse.cc

@@ -2393,6 +2393,7 @@ case SQLCOM_PREPARE:
     LEX_MASTER_INFO *lex_mi= &thd->lex->mi;
     Master_info *mi;
     bool new_master= 0;
+    bool master_info_added;
 
     if (check_global_access(thd, SUPER_ACL))
       goto error;
@@ -2415,14 +2416,18 @@ case SQLCOM_PREPARE:
       new_master= 1;
     }
 
-    res= change_master(thd, mi);
+    res= change_master(thd, mi, &master_info_added);
     if (res && new_master)
     {
       /*
-        The new master was added by change_master(). Remove it as it didn't
+        If the new master was added by change_master(), remove it as it didn't
-        work.
+        work (this will free mi as well).
+
+        If new master was not added, we still need to free mi.
       */
+      if (master_info_added)
         master_info_index->remove_master_info(&lex_mi->connection_name);
+      else
         delete mi;
     }
 

sql/sql_repl.cc

@@ -1677,10 +1677,14 @@ static bool get_string_parameter(char *to, const char *from, size_t length,
   @param mi Pointer to Master_info object belonging to the slave's IO
   thread.
 
+  @param master_info_added Out parameter saying if the Master_info *mi was
+  added to the global list of masters. This is useful in error conditions
+  to know if caller should free Master_info *mi.
+
   @retval FALSE success
   @retval TRUE error
 */
-bool change_master(THD* thd, Master_info* mi)
+bool change_master(THD* thd, Master_info* mi, bool *master_info_added)
 {
   int thread_mask;
   const char* errmsg= 0;
@@ -1695,6 +1699,7 @@ bool change_master(THD* thd, Master_info* mi)
   LEX_MASTER_INFO* lex_mi= &thd->lex->mi;
   DBUG_ENTER("change_master");
 
+  *master_info_added= false;
   /* 
     We need to check if there is an empty master_host. Otherwise
     change master succeeds, a master.info file is created containing 
@@ -1743,6 +1748,7 @@ bool change_master(THD* thd, Master_info* mi)
       ret= TRUE;
       goto err;
     }
+    *master_info_added= true;
   }
   if (global_system_variables.log_warnings > 1)
     sql_print_information("Master: '%.*s'  Master_info_file: '%s'  "

sql/sql_repl.h

@@ -41,7 +41,7 @@ extern my_bool opt_sporadic_binlog_dump_fail;
 
 int start_slave(THD* thd, Master_info* mi, bool net_report);
 int stop_slave(THD* thd, Master_info* mi, bool net_report);
-bool change_master(THD* thd, Master_info* mi);
+bool change_master(THD* thd, Master_info* mi, bool *master_info_added);
 bool mysql_show_binlog_events(THD* thd);
 int reset_slave(THD *thd, Master_info* mi);
 int reset_master(THD* thd);