From 78deb348f0d8579cccbac2afb1fd5ec982d2fd07 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 16 Jun 2006 12:29:04 +0200 Subject: [PATCH 1/4] BUG#20498: Wrong usage of hash_first() in get_open_table(). The get_open_table() function was using the wrong key for hash lookups. This could potentially lead to unpredictable behaviour. sql/ha_ndbcluster.cc: BUG#20498: Wrong usage of hash_first() in get_open_table(). --- sql/ha_ndbcluster.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sql/ha_ndbcluster.cc b/sql/ha_ndbcluster.cc index 2d623702670..2c9798369ed 100644 --- a/sql/ha_ndbcluster.cc +++ b/sql/ha_ndbcluster.cc @@ -317,7 +317,7 @@ byte *thd_ndb_share_get_key(THD_NDB_SHARE *thd_ndb_share, uint *length, my_bool not_used __attribute__((unused))) { *length= sizeof(thd_ndb_share->key); - return (byte*) thd_ndb_share->key; + return (byte*) &thd_ndb_share->key; } Thd_ndb::Thd_ndb() @@ -371,9 +371,9 @@ Thd_ndb::get_open_table(THD *thd, const void *key) DBUG_ENTER("Thd_ndb::get_open_table"); HASH_SEARCH_STATE state; THD_NDB_SHARE *thd_ndb_share= - (THD_NDB_SHARE*)hash_first(&open_tables, (byte *)key, sizeof(key), &state); + (THD_NDB_SHARE*)hash_first(&open_tables, (byte *)&key, sizeof(key), &state); while (thd_ndb_share && thd_ndb_share->key != key) - thd_ndb_share= (THD_NDB_SHARE*)hash_next(&open_tables, (byte *)key, sizeof(key), &state); + thd_ndb_share= (THD_NDB_SHARE*)hash_next(&open_tables, (byte *)&key, sizeof(key), &state); if (thd_ndb_share == 0) { thd_ndb_share= (THD_NDB_SHARE *) alloc_root(&thd->transaction.mem_root, From cb28cf8d88bf2a0e99fc37059d05c97dd1a904ec Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 19 Jun 2006 14:31:22 +0200 Subject: [PATCH 2/4] Fix a Valgrind leak error report for not freed binlog injector singleton. sql/mysqld.cc: Free the binlog injector singleton during shutdown. sql/rpl_injector.cc: Add free_instance() method. sql/rpl_injector.h: Add free_instance() method. --- sql/mysqld.cc | 7 +++++++ sql/rpl_injector.cc | 10 ++++++++++ sql/rpl_injector.h | 5 +++++ 3 files changed, 22 insertions(+) diff --git a/sql/mysqld.cc b/sql/mysqld.cc index ad6f7401965..87f3cd04605 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -28,6 +28,10 @@ #include "ha_myisam.h" +#ifdef HAVE_ROW_BASED_REPLICATION +#include "rpl_injector.h" +#endif + #ifdef WITH_INNOBASE_STORAGE_ENGINE #define OPT_INNODB_DEFAULT 1 #else @@ -1183,6 +1187,9 @@ void clean_up(bool print_message) what they have that is dependent on the binlog */ ha_binlog_end(current_thd); +#ifdef HAVE_ROW_BASED_REPLICATION + injector::free_instance(); +#endif mysql_bin_log.cleanup(); #ifdef HAVE_REPLICATION diff --git a/sql/rpl_injector.cc b/sql/rpl_injector.cc index 265f5f61213..012ff61e65e 100644 --- a/sql/rpl_injector.cc +++ b/sql/rpl_injector.cc @@ -155,6 +155,16 @@ injector *injector::instance() return s_injector; } +void injector::free_instance() +{ + injector *inj = s_injector; + + if (inj != 0) + { + s_injector= 0; + delete inj; + } +} injector::transaction injector::new_trans(THD *thd) diff --git a/sql/rpl_injector.h b/sql/rpl_injector.h index 14d5cca9b6c..c09e9753df3 100644 --- a/sql/rpl_injector.h +++ b/sql/rpl_injector.h @@ -59,6 +59,11 @@ public: */ static injector *instance(); + /* + Delete the singleton instance (if allocated). Used during server shutdown. + */ + static void free_instance(); + /* A transaction where rows can be added. From 860c104d87424cb140d937fa2a19a5d406e264e9 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 21 Jun 2006 12:27:24 +0200 Subject: [PATCH 3/4] BUG#20598 Fix race between cleanup and thread kill at server shutdown that would sometimes prevent proper cleanup, leading to Valgrind warnings. sql/mysqld.cc: Move logger cleanup to avoid races with thread kill. --- sql/mysqld.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 87f3cd04605..fb0ef81bda5 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -1266,13 +1266,13 @@ void clean_up(bool print_message) MYF(MY_WME | MY_FAE | MY_ALLOW_ZERO_PTR)); DBUG_PRINT("quit", ("Error messages freed")); /* Tell main we are ready */ + logger.cleanup_end(); (void) pthread_mutex_lock(&LOCK_thread_count); DBUG_PRINT("quit", ("got thread count lock")); ready_to_exit=1; /* do the broadcast inside the lock to ensure that my_end() is not called */ (void) pthread_cond_broadcast(&COND_thread_count); (void) pthread_mutex_unlock(&LOCK_thread_count); - logger.cleanup_end(); /* The following lines may never be executed as the main thread may have From 0f3cc95bf1523754d21cc3a4c59c0d107adc1c16 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 23 Jun 2006 14:50:02 +0200 Subject: [PATCH 4/4] BUG#20622: Fix one-byte buffer overrun in IM directory string handling. The problem was a call to convert_dirname() with a destination buffer that did not have room for the trailing slash added by that function. This could cause the instance manager to crash in some cases. mysys/mf_dirname.c: Clarify in comments that convert_dirname destination must be larger than source to accomodate a trailing slash. server-tools/instance-manager/instance_options.cc: Fix buffer overrun. --- mysys/mf_dirname.c | 4 +++- server-tools/instance-manager/instance_options.cc | 9 +++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/mysys/mf_dirname.c b/mysys/mf_dirname.c index 9206aa28078..4d78f039799 100644 --- a/mysys/mf_dirname.c +++ b/mysys/mf_dirname.c @@ -72,7 +72,9 @@ uint dirname_part(my_string to, const char *name) SYNPOSIS convert_dirname() - to Store result here + to Store result here. Must be at least of size + min(FN_REFLEN, strlen(from) + 1) to make room + for adding FN_LIBCHAR at the end. from Original filename from_end Pointer at end of filename (normally end \0) diff --git a/server-tools/instance-manager/instance_options.cc b/server-tools/instance-manager/instance_options.cc index 9389694822a..72621ed1662 100644 --- a/server-tools/instance-manager/instance_options.cc +++ b/server-tools/instance-manager/instance_options.cc @@ -391,8 +391,13 @@ int Instance_options::complete_initialization(const char *default_path, const char *tmp; char *end; - if (!mysqld_path && !(mysqld_path= strdup_root(&alloc, default_path))) - goto err; + if (!mysqld_path) + { + // Need one extra byte, as convert_dirname() adds a slash at the end. + if (!(mysqld_path= alloc_root(&alloc, strlen(default_path) + 2))) + goto err; + strcpy((char *)mysqld_path, default_path); + } // it's safe to cast this to char* since this is a buffer we are allocating end= convert_dirname((char*)mysqld_path, mysqld_path, NullS);