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.
