List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:November 15 2010 12:56pm
Subject:Re: bzr commit into mysql-5.5-bugteam branch (bar:3121) Bug#57306
View as plain text  
Hello,

Alexander Barkov a écrit, Le 15.11.2010 13:08:
> 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 is fine

> This way
> 
> - does not need mutex lock

Right, because set_query() already happens under some mutex, which SHOW 
PROCESSLIST already uses.

> - 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.

Uh, good catch.

>   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!

Yes, it's good to go.
Thread
bzr commit into mysql-5.5-bugteam branch (bar:3121) Bug#57306Alexander Barkov12 Nov
  • Re: bzr commit into mysql-5.5-bugteam branch (bar:3121) Bug#57306Guilhem Bichot12 Nov
    • Re: bzr commit into mysql-5.5-bugteam branch (bar:3121) Bug#57306Alexander Barkov12 Nov
      • Re: bzr commit into mysql-5.5-bugteam branch (bar:3121) Bug#57306Tor Didriksen15 Nov
        • Re: bzr commit into mysql-5.5-bugteam branch (bar:3121) Bug#57306Alexander Barkov15 Nov
          • Re: bzr commit into mysql-5.5-bugteam branch (bar:3121) Bug#57306Alexander Barkov15 Nov
            • Re: bzr commit into mysql-5.5-bugteam branch (bar:3121) Bug#57306Guilhem Bichot15 Nov
              • Re: bzr commit into mysql-5.5-bugteam branch (bar:3121) Bug#57306Alexander Barkov16 Nov
      • Re: bzr commit into mysql-5.5-bugteam branch (bar:3121) Bug#57306Guilhem Bichot15 Nov