Re: bk commit into 5.0 tree (aelkin:1.2526) BUG#26000

[Mats Kindahl]

Hi Andrei!

I was actually a little to fast: the patch actually *do* fix the 
problem. Please ignore my previous comments.

Patch is OK to push.

Just my few cents,
Mats Kindahl

Mats Kindahl wrote:
> Hi Andrei!
>
> Comments below.
>
> Just my few cents,
> Mats Kindahl
>
> Andrei Elkin wrote:
>> Below is the list of changes that have just been committed into a local
>> 5.0 repository of elkin. When elkin does a push these changes will
>> be propagated to the main repository and, within 24 hours after the
>> push, to the public repository.
>> For information on how to access the public repository
>> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>>
>> ChangeSet@stripped, 2007-09-26 21:59:17+02:00, aelkin@stripped 
>> +1 -0
>>   Bug #26000 SHOW SLAVE STATUS can crash mysqld during shutdown process
>>     active_mi has been reset (shutdown) at the time of quering with   
>> SHOW SLAVE STATUS so that   at handling of SHOW an attempt to read 
>> its members segfaults.
>>     Fixed with checking the value of active_mi before to call 
>> show_master_info()
>>   Merely send_ok() is invoked when active_mi does not exist.
>>   A test can not be easiely written.
>>     Notice, there are more analogical cases in the code which require 
>> a similar
>>   treatment (to be reported as a bug separately).
>>   sql/sql_parse.cc@stripped, 2007-09-26 21:59:11+02:00, 
>> aelkin@stripped +10 -1
>>     Ignore reporting and send only OK if master info struct has been 
>> destoyed.
>>     As this must be at shutdown merely a warning is sent to the client.
>>
>> diff -Nrup a/sql/sql_parse.cc b/sql/sql_parse.cc
>> --- a/sql/sql_parse.cc    2007-08-05 05:53:13 +02:00
>> +++ b/sql/sql_parse.cc    2007-09-26 21:59:11 +02:00
>> @@ -2844,7 +2844,16 @@ mysql_execute_command(THD *thd)
>>      if (check_global_access(thd, SUPER_ACL | REPL_CLIENT_ACL))
>>        goto error;
>>      pthread_mutex_lock(&LOCK_active_mi);
>> -    res = show_master_info(thd,active_mi);
>> +    if (active_mi != NULL)
>>   
>
> This won't solve the bug. It just moves the sweet spot to just after 
> the "if" above.
>
> To fix the problem, it is necessary to ensure that all read and writes 
> to the active_mi or the object that it points to is protected by the 
> LOCK_active_mi mutex.
>
>> +    {
>> +      res = show_master_info(thd, active_mi);
>> +    }
>> +    else
>> +    {
>> +      push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN, 0,
>> +                   "the master info structure does not exist");
>> +      send_ok(thd);
>> +    }
>>      pthread_mutex_unlock(&LOCK_active_mi);
>>      break;
>>    }
>>
>>   
>
>
> ------------------------------------------------------------------------
>
>
>   


-- 
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com