Re: bzr commit into mysql-5.1-bugteam branch (Alexey.Kopytov:2812) Bug#40552

[Alexey Kopytov]

Hi Sergei,

Sergei Golubchik wrote:
> Hi, Alexey!
> 
> On Feb 25, Alexey Kopytov wrote:
>> #At file:///Users/kaa/src/bzr/bugteam/bug40552/my51-bug40552/ based on
> revid:azundris@stripped
>>
>>  2812 Alexey Kopytov	2009-02-25
>>       Fix for bug #40552: Race condition around default_directories  
>>                           in load_defaults() 
>>        
>>       load_defaults(), my_search_option_files() and 
>>       my_print_default_files()  utilized a global variable 
>>       containing  a pointer to thread local memory. This could lead 
>>       to race conditions when those functions were called with high 
>>       concurrency. 
>>        
>>       Fixed by changing the interface of the said functions to avoid 
>>       the necessity for using a global variable.
> 
> Not a good idea. load_defaults() is de facto part of the API, we should
> try to avoid chaning the prototype.
> 

First, I don't see a way to avoid that w/o introducing new locks.

Second, I don't see how changing the load_defaults() prototype could 
affect anyone. Yes, it's a part of the mysys "API", but how many 
programs outside of our source tree use it? Please clarify why you think 
it's a bad idea.

Best regards,
Alexey.


> See how many files were affected by your small change in default.c
> 
>>       modified:
>>         client/mysql.cc
>>         client/mysql_upgrade.c
>>         client/mysqladmin.cc
>>         client/mysqlbinlog.cc
>>         client/mysqlcheck.c
>>         client/mysqldump.c
>>         client/mysqlimport.c
>>         client/mysqlshow.c
>>         client/mysqlslap.c
>>         client/mysqltest.cc
>>         extra/my_print_defaults.c
>>         include/my_sys.h
>>         mysys/default.c
>>         server-tools/instance-manager/instance_map.cc
>>         server-tools/instance-manager/options.cc
>>         server-tools/instance-manager/options.h
>>         sql-common/client.c
>>         sql/mysqld.cc
>>         storage/archive/archive_reader.c
>>         storage/myisam/myisamchk.c
>>         storage/myisam/myisampack.c
>>         storage/ndb/src/cw/cpcd/main.cpp
>>         storage/ndb/src/kernel/vm/Configuration.cpp
>>         storage/ndb/src/mgmclient/main.cpp
>>         storage/ndb/src/mgmsrv/InitConfigFileParser.cpp
>>         storage/ndb/src/mgmsrv/main.cpp
>>         storage/ndb/tools/delete_all.cpp
>>         storage/ndb/tools/desc.cpp
>>         storage/ndb/tools/drop_index.cpp
>>         storage/ndb/tools/drop_tab.cpp
>>         storage/ndb/tools/listTables.cpp
>>         storage/ndb/tools/ndb_config.cpp
>>         storage/ndb/tools/restore/restore_main.cpp
>>         storage/ndb/tools/select_all.cpp
>>         storage/ndb/tools/select_count.cpp
>>         storage/ndb/tools/waiter.cpp
>>         tests/mysql_client_test.c
>>
> Regards / Mit vielen Grüßen,
> Sergei
>