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=1
>>
>>
>
>
