Merge lp:~gl-az/percona-server/BT-23598-bug1167487-5.6 into lp:percona-server/5.6

[George Ormond Lorch III]

Merged from 5.5:
Fix for https://bugs.launchpad.net/percona-server/+bug/1167487 -Enhanced option modifiers do not deallocate memory | Valgrind: getopt_constraint_get_min_value and getopt_constraint_find definitely lost / still reachable

Originally this was just written off as a known, one time leak, which is is, but the failure causes scripted testing to fail and is really just sloppy.

To fix, a new path needed to be created from mysqld main() through my_cleanup and into the my_getopt.c to properly free the options modifiers structures created and saved during option parsing.
A new function was created called handle_options_end() which is called from cleanup() in mysqld.cc. handle_options_end() calls getopt_constraint_deinit() to signal it that it needs to free its internal static data.
getopt_constraint_init() used to contain a static hash table and initialization flag. These were moved out to global scope so that getopt_constraint_deinit() could have access to them.

While testing, a bad memory read/write was discovered in the min and max constraints which caused a crash during freeing of the constraints structs on shutdown. That has been fixed.

While testing, discovered a bug that caused incorrect behavior, reported as https://bugs.launchpad.net/percona-server/+bug/1233294 - certain option modifiers do not have effect
The my_option_constraints hash is hashed on the option name. Option names are normalized ('-' made '_') before inserting into the hash since 'finds' get called from both main where names have not been transformed and from plugins and other places where the names have been transformed.
The issue is that the searches were performed with non-normalized names, so the find function would not find a record for the same option name (innodb_io_capacity vs innodb-io-capacity). It would then create a new record, transform the name and attempt insertion, which would fail silently due to duplicate.
The new record would be returned and used for that particular constraint setting but would be leaked because it did not exist in the hash. The constraing is then not 'recorded' so any attempt to set outside of the constraint would succeed.
Fixed by transforming the name before looking up the constraint in the hash.

[George Ormond Lorch III (gl-az)]

http://jenkins.percona.com/view/PS%205.6/job/percona-server-5.6-param/309/

Same as mentioned in 5.5 MP, there are a few different failures from prior jenkins runs but have nothing to do w/ this fix. Please take a bit of a close look (I already did) to double check my work and make sure nothing was introduced.

[George Ormond Lorch III (gl-az)]

Same comments as 5.5

http://jenkins.percona.com/view/PS%205.6/job/percona-server-5.6-param/324/

[Laurynas Biveinis (laurynas-biveinis)]

Same comment as for 5.5 MP.

review: Approve

[George Ormond Lorch III (gl-az)]

New bug link added

[George Ormond Lorch III (gl-az)]

Same as 5.5, fixed up commit message and touched up MP description.

[Laurynas Biveinis (laurynas-biveinis)]

review: Approve

=== modified file 'Percona-Server/include/my_getopt.h'
--- Percona-Server/include/my_getopt.h	2013-08-06 15:16:34 +0000
+++ Percona-Server/include/my_getopt.h	2013-10-08 16:56:57 +0000
@@ -114,6 +114,7 @@
                               my_get_one_option,
                               const char **command_list);
 extern void my_cleanup_options(const struct my_option *options);
+extern void my_handle_options_end();
 extern void my_cleanup_options(const struct my_option *options);
 extern void my_print_help(const struct my_option *options);
 extern void my_print_variables(const struct my_option *options);

=== modified file 'Percona-Server/mysql-test/r/percona_enhanced_options_modifiers.result'
--- Percona-Server/mysql-test/r/percona_enhanced_options_modifiers.result	2012-08-07 18:49:16 +0000
+++ Percona-Server/mysql-test/r/percona_enhanced_options_modifiers.result	2013-10-08 16:56:57 +0000
@@ -112,9 +112,11 @@
 @@GLOBAL.INNODB_IO_CAPACITY
 150
 SET @@GLOBAL.INNODB_IO_CAPACITY=300;
+Warnings:
+Warning	1292	Truncated incorrect innodb_io_capacity value: '300'
 SELECT @@GLOBAL.INNODB_IO_CAPACITY;
 @@GLOBAL.INNODB_IO_CAPACITY
-300
+200
 SET @@GLOBAL.INNODB_IO_CAPACITY=160;
 SELECT @@GLOBAL.INNODB_IO_CAPACITY;
 @@GLOBAL.INNODB_IO_CAPACITY

=== modified file 'Percona-Server/mysys_ssl/my_getopt.cc'
--- Percona-Server/mysys_ssl/my_getopt.cc	2013-08-14 03:57:21 +0000
+++ Percona-Server/mysys_ssl/my_getopt.cc	2013-10-08 16:56:57 +0000
@@ -121,18 +121,19 @@
   my_free(moc);
 }
 
+static HASH my_option_constraints;
+static my_bool my_option_constraints_inited= FALSE;
+ 
 static HASH *getopt_constraint_init(my_bool create)
 {
-  static HASH my_option_constraints;
-  static int my_option_constraints_inited= 0;
-  
+ 
   if (!my_option_constraints_inited && create)
   {
     my_hash_init(&my_option_constraints, &my_charset_utf8_general_ci,
                  20, 0, 0,
                  (my_hash_get_key) getopt_constraint_get_name,
                  (void (*)(void *))getopt_constraint_free, HASH_UNIQUE);
-    my_option_constraints_inited= 1;
+    my_option_constraints_inited= TRUE;
     return &my_option_constraints;
   }
   else if (my_option_constraints_inited)
@@ -148,6 +149,8 @@
 {
   HASH *opts;
   struct my_option_constraint *moc;
+  char* pos;
+  char *normalized_name;
 
   opts= getopt_constraint_init(create);
   if (!opts)
@@ -155,28 +158,33 @@
   
   if (length == 0)
     length= strlen(name);
- 
+
+  normalized_name= my_strdup(name, MYF(MY_WME));
+  for (pos= normalized_name; *pos; pos++)
+  {
+    if (*pos == '-')
+      *pos= '_';
+  } 
+
   moc= (struct my_option_constraint *) my_hash_search(opts,
-                                                      (const uchar*) name,
+                                                      (const uchar*) normalized_name,
                                                       length);
 
   if (!moc && create)
   {
-    char* pos;
     moc= (struct my_option_constraint *) my_malloc(
                                    sizeof(struct my_option_constraint),
                                    MYF(MY_WME | MY_ZEROFILL));
 
-    moc->name= my_strdup(name, MYF(MY_WME));
-    for (pos= moc->name; *pos; pos++)
-    {
-       if (*pos == '-')
-         *pos= '_';
-    } 
+    moc->name= normalized_name; 
     moc->length= length;
-    
+
     my_hash_insert(opts, (uchar*) moc);
   }
+  else
+  {
+    my_free(normalized_name);
+  }
   return moc;
 }
 
@@ -191,7 +199,7 @@
     moc->min_value= my_malloc(create, MYF(MY_WME | MY_ZEROFILL));
 
   if (moc && moc->min_value)
-    return &moc->min_value;
+    return moc->min_value;
 
   return NULL;
 }
@@ -207,7 +215,7 @@
     moc->max_value= my_malloc(create, MYF(MY_WME | MY_ZEROFILL));
 
   if (moc && moc->max_value)
-    return &moc->max_value;
+    return moc->max_value;
 
   return NULL;
 }
@@ -867,6 +875,19 @@
 
 
 /*
+  Clean up any allocations made during handle_options on shutdown.
+*/
+void my_handle_options_end()
+{
+  if (my_option_constraints_inited)
+  {
+    my_hash_free(&my_option_constraints);
+    my_option_constraints_inited= FALSE;
+  }
+}
+
+
+/*
   function: check_struct_option
 
   Arguments: Current argument under processing from argv and a variable

=== modified file 'Percona-Server/sql/mysqld.cc'
--- Percona-Server/sql/mysqld.cc	2013-10-01 13:00:50 +0000
+++ Percona-Server/sql/mysqld.cc	2013-10-08 16:56:57 +0000
@@ -1915,6 +1915,8 @@
   if (THR_MALLOC)
     (void) pthread_key_delete(THR_MALLOC);
 
+  my_handle_options_end();
+
   /*
     The following lines may never be executed as the main thread may have
     killed us