From: Alexander Barkov
Date: November 15 2010 12:08pm
Subject: Re: bzr commit into mysql-5.5-bugteam branch (bar:3121) Bug#57306
List-Archive: http://lists.mysql.com/commits/123892
Message-Id: <4CE122A9.8000203@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Alexander Barkov wrote:
> Hi Tor, Guilhem,
> 
> Tor Didriksen wrote:
>>
>>     But I think there is no problem with copying CHARSET_INFO pointer.
>>     My understanding is that pointers use processor data types,
>>     which should be copied atomically, on both 32-bit and 64-bit 
>> hardware.
>>
>>
>>
>> There is no such guarantee, on any platform. It may, or may not work.
>> The only thing which is guaranteed to be atomic, is sig_atomic_t,
>> which intended for communication between signal handlers, and ordinary 
>> threads.
> 
> Thank you very much for bringing this issue up.
> After some researches I found the same information,
> that pointer reads/writes are not guaranteed to be atomic.
> It not only depends on hardware architecture but also on how
> compiler works
> (but I am pretty sure it will work fine with gcc and MS,
> on both 32/64bit platforms).
> 
> 
> To be on the safe side, I will put under mutex these pieces of the code:
> 
> - all updates for THD::variables::character_set_client
> - reads for THD::variables::character_set_client in
>   "SHOW PROCESSLIST" code in sql_show.cc

Dmitry proposed even a better solution:

add a new member into THD:

   CHARSET_INFO *query_charset;

and initialize it in set_query().


This way

- does not need mutex lock

- Sometimes stored procedures change character_set_client temporary,
   so using thd->variables.character_set_client is SHOW PROCESSS
   can be wrong when a thread is executing an SP.

   Using this extra member query_charset guarantees that character
   set we use in SHOW PROCESS lways matches to query_string content.

Thanks, Dmitry, for the idea!

> 
> 
> Other reads (outside of "SHOW PROCESSLIST" code) do not need to be
> locked, as all other reads done only by the THD itself and therefore
> can not interfere with any writes.
> 
> Thanks!
> 
>>
>> -- didrik
>>  
>>
>>
>>
>>         If there is a problem, maybe we should change the code which
>>         updates THD::variables::character_set_client to use
>>         LOCK_thd_data, or an atomic operation; and above we would use
>>         the mutex or the atomic operation...?
>>
>>
>>     I don't think we really need it.
>>
>>     
>>     --     MySQL Code Commits Mailing List
>>     For list archives: http://lists.mysql.com/commits
>>     To unsubscribe:       
>> http://lists.mysql.com/commits?unsub=didrik@stripped
>>
>>
> 
>