Bug #56709: Memory leaks at running the 5.1 test suite

Alexey Kopytov · Alexey Kopytov · commit 34148217b33c · 2010-09-22T23:33:18.000+04:00
Fixed a number of memory leaks discovered by valgrind.
--BZR--
revision-id: alexey.kopytov@sun.com-20100922193318-uc2aqfyqvhkdnqke
property-branch-nick: mysql-5.1-bugteam
property-file-info: ld7:file_id59:sp1f-dbug.c-19700101030959-dmu3qmh72hbptp5opqnmfgjtfckxi5ug7:message327:This is actually an addendum to the fix for bug #52629:
property-file-info: 
property-file-info: - there is no point in limiting the fix to just global
property-file-info: variables, session ones are also affected.
property-file-info: - zero all fields when allocating a new 'state' structure so
property-file-info: that FreeState() does not deal with unitialized data later.
property-file-info: - add a check for a NULL pointer in DBUGCloseFile()4:path11:dbug/dbug.ced7:file_id73:sp1f-partition_error.resu-20050720124221-iacosg62tkflqhizuvhtiem5oeicwjit7:message33:Added a test case for bug #56709.4:path35:mysql-test/r/partition_error.resulted7:file_id73:sp1f-variables_debug.resu-20080226150357-xrpcjxbbb6lhnyicksx3a7ozn3i5wh5x7:message33:Added a test case for bug #56709.4:path35:mysql-test/r/variables_debug.resulted7:file_id73:sp1f-partition_error.test-20050720124214-od2aou4vzloggrqktgmbjzvuhajiukpm7:message33:Added a test case for bug #56709.4:path33:mysql-test/t/partition_error.tested7:file_id73:sp1f-variables_debug.test-20080226150357-odgq22hsyfmfw7na43dwgeuppys77opf7:message33:Added a test case for bug #56709.4:path33:mysql-test/t/variables_debug.tested7:file_id69:sp1f-item_timefunc.cc-19700101030959-rvvlgmw5b4ewpuuxuntrkiqimyrr5sw27:message228:There is no point in declaring 'value' as a member of
property-file-info: Item_extract and dynamically allocating memory for it in
property-file-info: Item_extract::fix_length_and_dec(), since this string is only
property-file-info: used as a temporary storage in Item_extract::val_int().4:path20:sql/item_timefunc.cced7:file_id68:sp1f-item_timefunc.h-19700101030959-o34ypz6ggolzqmhgsjnqh6inkvgugi467:message55:Removed 'value' from the Item_extract class definition.4:path19:sql/item_timefunc.hed7:file_id64:sp1f-sql_load.cc-19700101030959-hoqlay5we4yslrw23xqedulkejw6a3o57:message304:- we may need to deallocate 'buffer' even when 'error' is
property-file-info:   non-zero in some cases, since 'error' is public, and there is
property-file-info:   external code modifying it.
property-file-info: - assign NULL to buffer when deallocating it so that we don't
property-file-info:   do it twice in the destructor
property-file-info: - there is no point in changing 'error' in the destructor.4:path15:sql/sql_load.ccee
testament3-sha1: aa5155cc8277df99850a4e80743c5ae7c98ea68c
diff --git a/dbug/dbug.c b/dbug/dbug.c
@@ -455,13 +455,8 @@ static void DbugParse(CODE_STATE *cs, const char *control)
   rel= control[0] == '+' || control[0] == '-';
   if ((!rel || (!stack->out_file && !stack->next)))
   {
-    /*
-      We need to free what's already in init_settings, because unlike
-      the thread related stack frames there's a chance that something
-      is in these variables already.
-    */
-    if (stack == &init_settings)
-      FreeState(cs, stack, 0);
+    /* Free memory associated with the state before resetting its members */
+    FreeState(cs, stack, 0);
     stack->flags= 0;
     stack->delay= 0;
     stack->maxdepth= 0;
@@ -1447,8 +1442,8 @@ static void PushState(CODE_STATE *cs)
   struct settings *new_malloc;
 
   new_malloc= (struct settings *) DbugMalloc(sizeof(struct settings));
+  bzero(new_malloc, sizeof(struct settings));
   new_malloc->next= cs->stack;
-  new_malloc->out_file= NULL;
   cs->stack= new_malloc;
 }
 
@@ -1957,7 +1952,7 @@ static FILE *OpenProfile(CODE_STATE *cs, const char *name)
 
 static void DBUGCloseFile(CODE_STATE *cs, FILE *fp)
 {
-  if (fp != stderr && fp != stdout && fclose(fp) == EOF)
+  if (fp != NULL && fp != stderr && fp != stdout && fclose(fp) == EOF)
   {
     pthread_mutex_lock(&THR_LOCK_dbug);
     (void) fprintf(cs->stack->out_file, ERR_CLOSE, cs->process);
diff --git a/mysql-test/r/partition_error.result b/mysql-test/r/partition_error.result
@@ -1008,4 +1008,14 @@ PARTITION p VALUES LESS THAN (1219089600),
 PARTITION pmax VALUES LESS THAN MAXVALUE);
 ERROR HY000: Constant, random or timezone-dependent expressions in (sub)partitioning function are not allowed
 DROP TABLE old;
+#
+# Bug #56709: Memory leaks at running the 5.1 test suite
+# 
+CREATE TABLE t1 (a TIMESTAMP NOT NULL PRIMARY KEY);
+ALTER TABLE t1
+PARTITION BY RANGE (EXTRACT(DAY FROM a)) (
+PARTITION p VALUES LESS THAN (18),
+PARTITION pmax VALUES LESS THAN MAXVALUE);
+ERROR HY000: Constant, random or timezone-dependent expressions in (sub)partitioning function are not allowed
+DROP TABLE t1;
 End of 5.1 tests
diff --git a/mysql-test/r/variables_debug.result b/mysql-test/r/variables_debug.result
@@ -24,4 +24,17 @@ SELECT @@global.debug;
 @@global.debug
 
 SET GLOBAL debug=@old_debug;
+#
+# Bug #56709: Memory leaks at running the 5.1 test suite
+# 
+SET @old_local_debug = @@debug;
+SET @@debug='d,foo';
+SELECT @@debug;
+@@debug
+d,foo
+SET @@debug='';
+SELECT @@debug;
+@@debug
+
+SET @@debug = @old_local_debug;
 End of 5.1 tests
diff --git a/mysql-test/t/partition_error.test b/mysql-test/t/partition_error.test
@@ -1252,4 +1252,18 @@ PARTITION pmax VALUES LESS THAN MAXVALUE);
 
 DROP TABLE old;
 
+--echo #
+--echo # Bug #56709: Memory leaks at running the 5.1 test suite
+--echo # 
+
+CREATE TABLE t1 (a TIMESTAMP NOT NULL PRIMARY KEY);
+
+--error ER_WRONG_EXPR_IN_PARTITION_FUNC_ERROR
+ALTER TABLE t1
+PARTITION BY RANGE (EXTRACT(DAY FROM a)) (
+PARTITION p VALUES LESS THAN (18),
+PARTITION pmax VALUES LESS THAN MAXVALUE);
+
+DROP TABLE t1;
+
 --echo End of 5.1 tests
diff --git a/mysql-test/t/variables_debug.test b/mysql-test/t/variables_debug.test
@@ -25,4 +25,17 @@ SELECT @@global.debug;
 
 SET GLOBAL debug=@old_debug;
 
+--echo #
+--echo # Bug #56709: Memory leaks at running the 5.1 test suite
+--echo # 
+
+SET @old_local_debug = @@debug;
+
+SET @@debug='d,foo';
+SELECT @@debug;
+SET @@debug='';
+SELECT @@debug;
+
+SET @@debug = @old_local_debug;
+
 --echo End of 5.1 tests
diff --git a/sql/item_timefunc.cc b/sql/item_timefunc.cc
@@ -2270,8 +2270,6 @@ void Item_extract::print(String *str, enum_query_type query_type)
 
 void Item_extract::fix_length_and_dec()
 {
-  value.alloc(32);				// alloc buffer
-
   maybe_null=1;					// If wrong date
   switch (int_type) {
   case INTERVAL_YEAR:		max_length=4; date_value=1; break;
@@ -2314,6 +2312,8 @@ longlong Item_extract::val_int()
   }
   else
   {
+    char buf[40];
+    String value(buf, sizeof(buf), &my_charset_bin);;
     String *res= args[0]->val_str(&value);
     if (!res || str_to_time_with_warn(res->ptr(), res->length(), &ltime))
     {
diff --git a/sql/item_timefunc.h b/sql/item_timefunc.h
@@ -702,7 +702,6 @@ class Item_date_add_interval :public Item_date_func
 
 class Item_extract :public Item_int_func
 {
-  String value;
   bool date_value;
  public:
   const interval_type int_type; // keep it public
diff --git a/sql/sql_load.cc b/sql/sql_load.cc
@@ -1116,6 +1116,7 @@ READ_INFO::READ_INFO(File file_par, uint tot_length, CHARSET_INFO *cs,
 		      MYF(MY_WME)))
     {
       my_free((uchar*) buffer,MYF(0)); /* purecov: inspected */
+      buffer= NULL;
       error=1;
     }
     else
@@ -1142,13 +1143,10 @@ READ_INFO::READ_INFO(File file_par, uint tot_length, CHARSET_INFO *cs,
 
 READ_INFO::~READ_INFO()
 {
-  if (!error)
-  {
-    if (need_end_io_cache)
-      ::end_io_cache(&cache);
-    my_free((uchar*) buffer,MYF(0));
-    error=1;
-  }
+  if (!error && need_end_io_cache)
+    ::end_io_cache(&cache);
+
+  my_free(buffer, MYF(MY_ALLOW_ZERO_PTR));
 }
 
 

Original file line number	Diff line number	Diff line change
`@@ -2270,8 +2270,6 @@ void Item_extract::print(String *str, enum_query_type query_type)`
`2270`	`2270`
`2271`	`2271`	`void Item_extract::fix_length_and_dec()`
`2272`	`2272`	`{`
`2273`		`- value.alloc(32); // alloc buffer`
`2274`		`-`
`2275`	`2273`	`maybe_null=1; // If wrong date`
`2276`	`2274`	`switch (int_type) {`
`2277`	`2275`	`case INTERVAL_YEAR: max_length=4; date_value=1; break;`
`@@ -2314,6 +2312,8 @@ longlong Item_extract::val_int()`
`2314`	`2312`	`}`
`2315`	`2313`	`else`
`2316`	`2314`	`{`
	`2315`	`+ char buf[40];`
	`2316`	`+ String value(buf, sizeof(buf), &my_charset_bin);;`
`2317`	`2317`	`String *res= args[0]->val_str(&value);`
`2318`	`2318`	`if (!res \|\| str_to_time_with_warn(res->ptr(), res->length(), &ltime))`
`2319`	`2319`	`{`
Original file line number	Diff line number	Diff line change
`@@ -702,7 +702,6 @@ class Item_date_add_interval :public Item_date_func`
`702`	`702`
`703`	`703`	`class Item_extract :public Item_int_func`
`704`	`704`	`{`
`705`		`- String value;`
`706`	`705`	`bool date_value;`
`707`	`706`	`public:`
`708`	`707`	`const interval_type int_type; // keep it public`
Original file line number	Diff line number	Diff line change
`@@ -1116,6 +1116,7 @@ READ_INFO::READ_INFO(File file_par, uint tot_length, CHARSET_INFO *cs,`
`1116`	`1116`	`MYF(MY_WME)))`
`1117`	`1117`	`{`
`1118`	`1118`	`my_free((uchar) buffer,MYF(0)); / purecov: inspected */`
	`1119`	`+ buffer= NULL;`
`1119`	`1120`	`error=1;`
`1120`	`1121`	`}`
`1121`	`1122`	`else`
`@@ -1142,13 +1143,10 @@ READ_INFO::READ_INFO(File file_par, uint tot_length, CHARSET_INFO *cs,`
`1142`	`1143`
`1143`	`1144`	`READ_INFO::~READ_INFO()`
`1144`	`1145`	`{`
`1145`		`- if (!error)`
`1146`		`- {`
`1147`		`- if (need_end_io_cache)`
`1148`		`- ::end_io_cache(&cache);`
`1149`		`- my_free((uchar*) buffer,MYF(0));`
`1150`		`- error=1;`
`1151`		`- }`
	`1146`	`+ if (!error && need_end_io_cache)`
	`1147`	`+ ::end_io_cache(&cache);`
	`1148`	`+`
	`1149`	`+ my_free(buffer, MYF(MY_ALLOW_ZERO_PTR));`
`1152`	`1150`	`}`
`1153`	`1151`
`1154`	`1152`