From 8969530cffde0c7a466cf3aa8c4b7462d0ace2d2 Mon Sep 17 00:00:00 2001 From: Staale Smedseng Date: Sat, 12 Dec 2009 19:11:25 +0100 Subject: Bug #45058 init_available_charsets uses double checked locking As documented in the bug report, the double checked locking pattern has inherent issues, and cannot guarantee correct initialization. This patch replaces the logic in init_available_charsets() with the use of pthread_once(3). A wrapper function, my_pthread_once(), is introduced and is used in lieu of direct calls to init_available_charsets(). Related defines MY_PTHREAD_ONCE_* are also introduced. For the Windows platform, the implementation in lp:sysbench is ported. For single-thread use, a simple define calls the function and sets the pthread_once control variable. Charset initialization is modified to use my_pthread_once(). include/my_no_pthread.h: Dummy my_pthread_once() for single thread use. include/my_pthread.h: Declaration for new function my_pthread_once(). mysys/charset.c: Logic in init_available_charsets() is simplified. Using my_pthread_once() for all calls to this func. mysys/my_winthread.c: Windows implementation of my_pthread_once(). --- mysys/charset.c | 77 +++++++++++++++++----------------------------------- mysys/my_init.c | 1 - mysys/my_winthread.c | 32 ++++++++++++++++++++++ 3 files changed, 57 insertions(+), 53 deletions(-) (limited to 'mysys') diff --git a/mysys/charset.c b/mysys/charset.c index b23ab084e90..d59be4ab6c7 100644 --- a/mysys/charset.c +++ b/mysys/charset.c @@ -321,7 +321,6 @@ static int add_collation(CHARSET_INFO *cs) #define MY_CHARSET_INDEX "Index.xml" const char *charsets_dir= NULL; -static int charset_initialized=0; static my_bool my_read_charset_file(const char *filename, myf myflags) @@ -399,63 +398,37 @@ static void *cs_alloc(size_t size) } -#ifdef __NETWARE__ -my_bool STDCALL init_available_charsets(myf myflags) -#else -static my_bool init_available_charsets(myf myflags) -#endif +static my_pthread_once_t charsets_initialized= MY_PTHREAD_ONCE_INIT; + +static void init_available_charsets(void) { char fname[FN_REFLEN + sizeof(MY_CHARSET_INDEX)]; - my_bool error=FALSE; - /* - We have to use charset_initialized to not lock on THR_LOCK_charset - inside get_internal_charset... - */ - if (!charset_initialized) + CHARSET_INFO **cs; + + bzero(&all_charsets,sizeof(all_charsets)); + init_compiled_charsets(MYF(0)); + + /* Copy compiled charsets */ + for (cs=all_charsets; + cs < all_charsets+array_elements(all_charsets)-1 ; + cs++) { - CHARSET_INFO **cs; - /* - To make things thread safe we are not allowing other threads to interfere - while we may changing the cs_info_table - */ - pthread_mutex_lock(&THR_LOCK_charset); - if (!charset_initialized) + if (*cs) { - bzero(&all_charsets,sizeof(all_charsets)); - init_compiled_charsets(myflags); - - /* Copy compiled charsets */ - for (cs=all_charsets; - cs < all_charsets+array_elements(all_charsets)-1 ; - cs++) - { - if (*cs) - { - if (cs[0]->ctype) - if (init_state_maps(*cs)) - *cs= NULL; - } - } - - strmov(get_charsets_dir(fname), MY_CHARSET_INDEX); - error= my_read_charset_file(fname,myflags); - charset_initialized=1; + if (cs[0]->ctype) + if (init_state_maps(*cs)) + *cs= NULL; } - pthread_mutex_unlock(&THR_LOCK_charset); } - return error; -} - - -void free_charsets(void) -{ - charset_initialized=0; + + strmov(get_charsets_dir(fname), MY_CHARSET_INDEX); + my_read_charset_file(fname, MYF(0)); } uint get_collation_number(const char *name) { - init_available_charsets(MYF(0)); + my_pthread_once(&charsets_initialized, init_available_charsets); return get_collation_number_internal(name); } @@ -463,7 +436,7 @@ uint get_collation_number(const char *name) uint get_charset_number(const char *charset_name, uint cs_flags) { CHARSET_INFO **cs; - init_available_charsets(MYF(0)); + my_pthread_once(&charsets_initialized, init_available_charsets); for (cs= all_charsets; cs < all_charsets+array_elements(all_charsets)-1 ; @@ -480,7 +453,7 @@ uint get_charset_number(const char *charset_name, uint cs_flags) const char *get_charset_name(uint charset_number) { CHARSET_INFO *cs; - init_available_charsets(MYF(0)); + my_pthread_once(&charsets_initialized, init_available_charsets); cs=all_charsets[charset_number]; if (cs && (cs->number == charset_number) && cs->name ) @@ -538,7 +511,7 @@ CHARSET_INFO *get_charset(uint cs_number, myf flags) if (cs_number == default_charset_info->number) return default_charset_info; - (void) init_available_charsets(MYF(0)); /* If it isn't initialized */ + my_pthread_once(&charsets_initialized, init_available_charsets); if (!cs_number || cs_number >= array_elements(all_charsets)-1) return NULL; @@ -560,7 +533,7 @@ CHARSET_INFO *get_charset_by_name(const char *cs_name, myf flags) { uint cs_number; CHARSET_INFO *cs; - (void) init_available_charsets(MYF(0)); /* If it isn't initialized */ + my_pthread_once(&charsets_initialized, init_available_charsets); cs_number=get_collation_number(cs_name); cs= cs_number ? get_internal_charset(cs_number,flags) : NULL; @@ -585,7 +558,7 @@ CHARSET_INFO *get_charset_by_csname(const char *cs_name, DBUG_ENTER("get_charset_by_csname"); DBUG_PRINT("enter",("name: '%s'", cs_name)); - (void) init_available_charsets(MYF(0)); /* If it isn't initialized */ + my_pthread_once(&charsets_initialized, init_available_charsets); cs_number= get_charset_number(cs_name, cs_flags); cs= cs_number ? get_internal_charset(cs_number, flags) : NULL; diff --git a/mysys/my_init.c b/mysys/my_init.c index a60927be693..453c72f999f 100644 --- a/mysys/my_init.c +++ b/mysys/my_init.c @@ -165,7 +165,6 @@ void my_end(int infoflag) my_print_open_files(); } } - free_charsets(); my_error_unregister_all(); my_once_free(); diff --git a/mysys/my_winthread.c b/mysys/my_winthread.c index e94369bec32..ef2a20c2ddc 100644 --- a/mysys/my_winthread.c +++ b/mysys/my_winthread.c @@ -137,4 +137,36 @@ int win_pthread_setspecific(void *a,void *b,uint length) return 0; } + +/* + One time initialization. For simplicity, we assume initializer thread + does not exit within init_routine(). +*/ +int my_pthread_once(my_pthread_once_t *once_control, + void (*init_routine)(void)) +{ + LONG state= InterlockedCompareExchange(once_control, MY_PTHREAD_ONCE_INPROGRESS, + MY_PTHREAD_ONCE_INIT); + switch(state) + { + case MY_PTHREAD_ONCE_INIT: + /* This is initializer thread */ + (*init_routine)(); + *once_control= MY_PTHREAD_ONCE_DONE; + break; + + case MY_PTHREAD_ONCE_INPROGRESS: + /* init_routine in progress. Wait for its completion */ + while(*once_control == MY_PTHREAD_ONCE_INPROGRESS) + { + Sleep(1); + } + break; + case MY_PTHREAD_ONCE_DONE: + /* Nothing to do */ + break; + } + return 0; +} + #endif -- cgit v1.2.1 From cff23162ec4bfc0c0fa489eb8568de0a3ccdd1ab Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Tue, 15 Dec 2009 13:48:29 +0400 Subject: Bug#49134 5.1 server segfaults with 2byte collation file Problem: add_collation did not check that cs->number is smaller than the number of elements in the array all_charsets[], so server could crash when loading an Index.xml file with a collation ID greater the number of elements (for example when downgrading from 5.5). Fix: adding a condition to check that cs->number is not out of valid range. --- mysys/charset.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'mysys') diff --git a/mysys/charset.c b/mysys/charset.c index d59be4ab6c7..b1b91d716ba 100644 --- a/mysys/charset.c +++ b/mysys/charset.c @@ -220,7 +220,8 @@ copy_uca_collation(CHARSET_INFO *to, CHARSET_INFO *from) static int add_collation(CHARSET_INFO *cs) { if (cs->name && (cs->number || - (cs->number=get_collation_number_internal(cs->name)))) + (cs->number=get_collation_number_internal(cs->name))) && + cs->number < array_elements(all_charsets)) { if (!all_charsets[cs->number]) { -- cgit v1.2.1 From cf9966f86f3c6245f33342f2f5498a0c5efa96e4 Mon Sep 17 00:00:00 2001 From: Satya B Date: Thu, 17 Dec 2009 16:55:50 +0530 Subject: Fix for Bug#37408 - Compressed MyISAM files should not require/use mmap() When compressed myisam files are opened, they are always memory mapped sometimes causing memory swapping problems. When we mmap the myisam compressed tables of size greater than the memory available, the kswapd0 process utilization is very high consuming 30-40% of the cpu. This happens only with linux kernels older than 2.6.9 With newer linux kernels, we don't have this problem of high cpu consumption and this option may not be required. The option 'myisam_mmap_size' is added to limit the amount of memory used for memory mapping of myisam files. This option is not dynamic. The default value on 32 bit system is 4294967295 bytes and on 64 bit system it is 18446744073709547520 bytes. Note: Testcase only tests the option variable. The actual bug has be to tested manually. include/my_global.h: Fix for Bug #37408 - Compressed MyISAM files should not require/use mmap() define SIZE_T_MAX include/myisam.h: Fix for Bug #37408 - Compressed MyISAM files should not require/use mmap() declare 'myisam_mmap_size' and 'myisam_mmap_used' variables and the mutex THR_LOCK_myisam_mmap myisam/mi_packrec.c: Fix for Bug #37408 - Compressed MyISAM files should not require/use mmap() add 'myisam_mmap_size' option which limits the memory available to mmap of myisam files myisam/mi_static.c: Fix for Bug #37408 - Compressed MyISAM files should not require/use mmap() declare 'myisam_mmap_size' and 'myisam_mmap_used' variables and the mutex THR_LOCK_myisam_mmap myisam/myisamdef.h: Fix for Bug #37408 - Compressed MyISAM files should not require/use mmap() move MEMMAP_EXTRA_MARGIN to myisam.h so that it can be used in mysqld.cc mysql-test/r/variables.result: Fix for Bug #37408 - Compressed MyISAM files should not require/use mmap() Testcase for BUG#37408 to test the myisam_mmap_size option mysql-test/t/variables.test: Fix for Bug #37408 - Compressed MyISAM files should not require/use mmap() Testcase for BUG#37408 to test the myisam_mmap_size option mysys/my_thr_init.c: Fix for Bug #37408 - Compressed MyISAM files should not require/use mmap() intialize the lock THR_LOCK_myisam_mmap sql/mysqld.cc: Fix for Bug #37408 - Compressed MyISAM files should not require/use mmap() add the 'myisam_mmap_size' option sql/set_var.cc: Fix for Bug #37408 - Compressed MyISAM files should not require/use mmap() add the 'myisam_mmap_size' to the SHOW VARIABLES list --- mysys/my_thr_init.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'mysys') diff --git a/mysys/my_thr_init.c b/mysys/my_thr_init.c index 64fc99952e9..8dcffaebbbb 100644 --- a/mysys/my_thr_init.c +++ b/mysys/my_thr_init.c @@ -30,7 +30,9 @@ pthread_key(struct st_my_thread_var, THR_KEY_mysys); #endif /* USE_TLS */ pthread_mutex_t THR_LOCK_malloc,THR_LOCK_open, THR_LOCK_lock,THR_LOCK_isam,THR_LOCK_myisam,THR_LOCK_heap, - THR_LOCK_net, THR_LOCK_charset, THR_LOCK_threads; + THR_LOCK_net, THR_LOCK_charset, THR_LOCK_threads, + THR_LOCK_myisam_mmap; + pthread_cond_t THR_COND_threads; uint THR_thread_count= 0; uint my_thread_end_wait_time= 5; @@ -143,6 +145,7 @@ my_bool my_thread_global_init(void) pthread_mutex_init(&THR_LOCK_lock,MY_MUTEX_INIT_FAST); pthread_mutex_init(&THR_LOCK_isam,MY_MUTEX_INIT_SLOW); pthread_mutex_init(&THR_LOCK_myisam,MY_MUTEX_INIT_SLOW); + pthread_mutex_init(&THR_LOCK_myisam_mmap,MY_MUTEX_INIT_FAST); pthread_mutex_init(&THR_LOCK_heap,MY_MUTEX_INIT_FAST); pthread_mutex_init(&THR_LOCK_net,MY_MUTEX_INIT_FAST); pthread_mutex_init(&THR_LOCK_charset,MY_MUTEX_INIT_FAST); @@ -208,6 +211,7 @@ void my_thread_global_end(void) pthread_mutex_destroy(&THR_LOCK_lock); pthread_mutex_destroy(&THR_LOCK_isam); pthread_mutex_destroy(&THR_LOCK_myisam); + pthread_mutex_destroy(&THR_LOCK_myisam_mmap); pthread_mutex_destroy(&THR_LOCK_heap); pthread_mutex_destroy(&THR_LOCK_net); pthread_mutex_destroy(&THR_LOCK_charset); -- cgit v1.2.1 From 06a1df91813ea2c39f7312bcf8af972c7e8a926f Mon Sep 17 00:00:00 2001 From: Davi Arnaut Date: Thu, 17 Dec 2009 15:58:38 -0200 Subject: Bug#48983: Bad strmake calls (length one too long) The problem is a somewhat common misusage of the strmake function. The strmake(dst, src, len) function writes at most /len/ bytes to the string pointed to by src, not including the trailing null byte. Hence, if /len/ is the exact length of the destination buffer, a one byte buffer overflow can occur if the length of the source string is equal to or greater than /len/. client/mysqldump.c: Make room for the trailing null byte. libmysql/libmysql.c: Add comment, there is enough room in the buffer. Increase buffer length, two strings are concatenated. libmysqld/lib_sql.cc: Make room for the trailing null byte. mysys/default.c: Make room for the trailing null bytes. mysys/mf_pack.c: Make room for the trailing null byte. server-tools/instance-manager/commands.cc: Copy only if overflow isn't possible in both cases. server-tools/instance-manager/listener.cc: Make room for the trailing null byte. sql/log.cc: Make room for the trailing null byte. sql/sp_pcontext.h: Cosmetic fix. sql/sql_acl.cc: MAX_HOSTNAME already specifies space for the trailing null byte. sql/sql_parse.cc: Make room for the trailing null byte. sql/sql_table.cc: Make room for the trailing null byte. --- mysys/default.c | 2 +- mysys/mf_pack.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'mysys') diff --git a/mysys/default.c b/mysys/default.c index 362aa0d4605..0d162dc13d5 100644 --- a/mysys/default.c +++ b/mysys/default.c @@ -605,7 +605,7 @@ static int search_default_file_with_ext(Process_option_func opt_handler, int recursion_level) { char name[FN_REFLEN + 10], buff[4096], curr_gr[4096], *ptr, *end, **tmp_ext; - char *value, option[4096], tmp[FN_REFLEN]; + char *value, option[4096+2], tmp[FN_REFLEN]; static const char includedir_keyword[]= "includedir"; static const char include_keyword[]= "include"; const int max_recursion_level= 10; diff --git a/mysys/mf_pack.c b/mysys/mf_pack.c index d2bac95b391..3053699f457 100644 --- a/mysys/mf_pack.c +++ b/mysys/mf_pack.c @@ -234,7 +234,7 @@ my_bool my_use_symdir=0; /* Set this if you want to use symdirs */ #ifdef USE_SYMDIR void symdirget(char *dir) { - char buff[FN_REFLEN]; + char buff[FN_REFLEN+1]; char *pos=strend(dir); if (dir[0] && pos[-1] != FN_DEVCHAR && my_access(dir, F_OK)) { @@ -246,7 +246,7 @@ void symdirget(char *dir) *pos++=temp; *pos=0; /* Restore old filename */ if (file >= 0) { - if ((length= my_read(file, buff, sizeof(buff), MYF(0))) > 0) + if ((length= my_read(file, buff, sizeof(buff) - 1, MYF(0))) > 0) { for (pos= buff + length ; pos > buff && (iscntrl(pos[-1]) || isspace(pos[-1])) ; -- cgit v1.2.1 From 132b46e96eb48b8d9d993583ed7b9ecd87e53674 Mon Sep 17 00:00:00 2001 From: Magne Mahre Date: Thu, 21 Jan 2010 09:10:05 +0100 Subject: WL#5154 Remove deprecated 4.1 features Several items said to be deprecated in the 4.1 manual have never been removed. This worklog adds deprecation warnings when these items are used, and warns the user that the items will be removed in MySQL 5.6. A couple of previously deprecation decision have been reversed (see single file comments) client/client_priv.h: Macro similar to the one in the server (mysql_priv.h) for printing a deprecation warning message client/mysql.cc: no-auto-rehash will not be deprecated skip-line-numbers will not be deprecated skip-column-names will not be deprecated no-pager is deprecated set-variable is deprecated no-named-commands is deprecated client/mysqladmin.cc: set-variable is deprecated client/mysqlbinlog.cc: position is deprecated client/mysqldump.c: first-slave is deprecated no-set-names is deprecated set-variable is deprecated mysql-test/r/mysqlbinlog.result: Adding the [Warning] to the test case, just to show that the deprecation works. The test case will be changed in Celosia to use --start-position. mysys/my_getopt.c: set-variable (include -O) is deprecated scripts/mysqld_multi.sh: Warning for mysqld_multi sql/mysqld.cc: default-collation is deprecated log-bin-trust-routine-creators is deprecated set-variable is deprecated default-character-set is deprecated safe-show-database is deprecated sql/share/errmsg.txt: Added version number for sql_log_update deprecation message. --- mysys/my_getopt.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'mysys') diff --git a/mysys/my_getopt.c b/mysys/my_getopt.c index e561d74f4d1..fc5662812b0 100644 --- a/mysys/my_getopt.c +++ b/mysys/my_getopt.c @@ -146,6 +146,10 @@ int handle_options(int *argc, char ***argv, { /* --set-variable, or -O */ if (*cur_arg == 'O') { + my_getopt_error_reporter(WARNING_LEVEL, + "%s: Option '-O' is deprecated. " + "Use --variable-name=value instead.", + my_progname); must_be_var= 1; if (!(*++cur_arg)) /* If not -Ovar=# */ @@ -165,6 +169,11 @@ int handle_options(int *argc, char ***argv, } else if (!getopt_compare_strings(cur_arg, "-set-variable", 13)) { + my_getopt_error_reporter(WARNING_LEVEL, + "%s: Option '--set-variable' is deprecated. " + "Use --variable-name=value instead.", + my_progname); + must_be_var= 1; if (cur_arg[13] == '=') { -- cgit v1.2.1 From 292f6568fa377420c81e0317a26b804057ce208c Mon Sep 17 00:00:00 2001 From: Michael Widenius Date: Tue, 9 Mar 2010 21:22:24 +0200 Subject: Added count of my_sync calls (to SHOW STATUS) tmp_table_size can now be set to 0 (to disable in memory internal temp tables) Improved speed for internal Maria temp tables: - Don't use packed keys, except with long text fields. - Don't copy key all accessed pages during key search. Some new benchmark tests to sql-bench (for group by) BUILD/compile-pentium64-gcov: Update script to use same pentium_config flags as other tests BUILD/compile-pentium64-gprof: Update script to use same pentium_config flags as other tests include/my_sys.h: Added count of my_sync calls mysql-test/r/variables.result: tmp_table_size can now be set to 0 sql-bench/test-select.sh: Added some new test for GROUP BY on a not key field and group by with different order by sql/mysqld.cc: Added count of my_sync calls tmp_table_size can now be set to 0 (to disable in memory internal temp tables) sql/sql_select.cc: If tmp_table_size is 0, don't use in memory temp tables (good for benchmarking MyISAM/Maria temp tables) Don't pack keys for Maria tables; The 8K page size makes packed keys too slow for temp tables. storage/maria/ma_key_recover.h: Moved definition to maria_def.h storage/maria/ma_page.c: Moved code used to simplify comparing of identical Maria tables to own function (page_cleanup()) Fixed that one can read a page with a read lock. storage/maria/ma_rkey.c: For not exact key reads, cache the page where we found key (to speed up future read-next/read-prev calls) storage/maria/ma_search.c: Moved code to cache last key page to separate function. Instead of copying pages, only get a link to the page. This notable speeds up key searches on bigger tables. storage/maria/ma_write.c: Added comment storage/maria/maria_def.h: Moved page_cleanup() to separate function. --- mysys/my_sync.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'mysys') diff --git a/mysys/my_sync.c b/mysys/my_sync.c index f8961202fa3..967a6ae6c78 100644 --- a/mysys/my_sync.c +++ b/mysys/my_sync.c @@ -17,6 +17,8 @@ #include "mysys_err.h" #include +ulong my_sync_count; /* Count number of sync calls */ + /* Sync data in file to disk @@ -46,6 +48,7 @@ int my_sync(File fd, myf my_flags) DBUG_ENTER("my_sync"); DBUG_PRINT("my",("fd: %d my_flags: %d", fd, my_flags)); + statistic_increment(my_sync_count,&THR_LOCK_open); do { #if defined(F_FULLFSYNC) -- cgit v1.2.1