MySQL Lists are EOL. Please join:

List:Internals« Previous MessageNext Message »
From:Petr Chardin Date:August 25 2005 11:34am
Subject:bk commit into 5.0 tree (petr:1.2000) BUG#11333
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of cps. When cps does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet
  1.2000 05/08/25 15:34:14 petr@stripped +5 -0
  Fix Bug#11333 "Stored Procedure: Memory blow up on repeated SELECT ... INTO query"

  sql/sp_head.cc
    1.169 05/08/25 15:34:07 petr@stripped +19 -4
    String allocation should be done on the system heap for now.

  sql/item.h
    1.162 05/08/25 15:34:07 petr@stripped +18 -0
    Add new method and constructor for Item_string.

  sql/item.cc
    1.163 05/08/25 15:34:07 petr@stripped +1 -0
    we should call destructors for Items before reuse

  mysql-test/t/sp.test
    1.140 05/08/25 15:34:07 petr@stripped +34 -0
    Add test for Bug #11333 "Stored Procedure: Memory blow up on repeated SELECT ... INTO query"

  mysql-test/r/sp.result
    1.145 05/08/25 15:34:07 petr@stripped +19 -0
    update result

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	petr
# Host:	owlet.
# Root:	/home/cps/mysql/devel/mysql-5.0-sp11333

--- 1.162/sql/item.cc	2005-08-12 19:04:48 +04:00
+++ 1.163/sql/item.cc	2005-08-25 15:34:07 +04:00
@@ -303,6 +303,7 @@
     if (rsize)
       (*rsize)= reuse->rsize;
     reuse->cleanup();
+    delete reuse;
     TRASH((void *)reuse, size);
     return (void *)reuse;
   }

--- 1.161/sql/item.h	2005-08-19 16:22:26 +04:00
+++ 1.162/sql/item.h	2005-08-25 15:34:07 +04:00
@@ -1298,6 +1298,15 @@
     // it is constant => can be used without fix_fields (and frequently used)
     fixed= 1;
   }
+  /* Just create an item and do not fill string representation */
+  Item_string(CHARSET_INFO *cs, Derivation dv= DERIVATION_COERCIBLE)
+  {
+    collation.set(cs, dv);
+    max_length= 0;
+    set_name(NULL, 0, cs);
+    decimals= NOT_FIXED_DEC;
+    fixed= 1;
+  }
   Item_string(const char *name_par, const char *str, uint length,
 	      CHARSET_INFO *cs, Derivation dv= DERIVATION_COERCIBLE)
   {
@@ -1308,6 +1317,15 @@
     decimals=NOT_FIXED_DEC;
     // it is constant => can be used without fix_fields (and frequently used)
     fixed= 1;
+  }
+  /*
+    This is used in stored procedures to avoid memory leaks and
+    does a deep copy of its argument.
+  */
+  void set_str_with_copy(const char *str_arg, uint length_arg)
+  {
+    str_value.copy(str_arg, length_arg, collation.collation);
+    max_length= str_value.numchars() * collation.collation->mbmaxlen;
   }
   enum Type type() const { return STRING_ITEM; }
   double val_real();

--- 1.144/mysql-test/r/sp.result	2005-08-19 17:03:12 +04:00
+++ 1.145/mysql-test/r/sp.result	2005-08-25 15:34:07 +04:00
@@ -3166,4 +3166,23 @@
 4
 truncate t2|
 drop procedure if exists bug12168|
+drop table if exists t3|
+drop procedure if exists bug11333|
+create table t3 (c1 char(128))|
+insert into t3 values 
+('AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA')|
+create procedure bug11333(i int)
+begin
+declare tmp varchar(128);
+set @x = 0;
+repeat
+select c1 into tmp from t3
+where c1 = 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA';
+set @x = @x + 1;
+until @x >= i
+end repeat;
+end|
+call bug11333(10)|
+drop procedure bug11333|
+drop table t3|
 drop table t1,t2;

--- 1.139/mysql-test/t/sp.test	2005-08-19 17:03:12 +04:00
+++ 1.140/mysql-test/t/sp.test	2005-08-25 15:34:07 +04:00
@@ -3996,6 +3996,40 @@
 drop procedure if exists bug12168|
 
 #
+# Bug #11333 "Stored Procedure: Memory blow up on repeated SELECT ... INTO
+# query"
+# One more memleak bug. Use the test to check memory consumption.
+#
+
+--disable_warnings
+drop table if exists t3|
+drop procedure if exists bug11333|
+--enable_warnings
+
+create table t3 (c1 char(128))|
+
+insert into t3 values 
+  ('AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA')|
+
+
+create procedure bug11333(i int)
+begin
+    declare tmp varchar(128);
+    set @x = 0;
+    repeat
+        select c1 into tmp from t3
+          where c1 = 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA';
+        set @x = @x + 1;
+        until @x >= i
+    end repeat;
+end|
+
+call bug11333(10)|
+
+drop procedure bug11333|
+drop table t3|
+
+#
 # BUG#NNNN: New bug synopsis
 #
 #--disable_warnings

--- 1.168/sql/sp_head.cc	2005-08-19 17:03:12 +04:00
+++ 1.169/sql/sp_head.cc	2005-08-25 15:34:07 +04:00
@@ -249,10 +249,25 @@
         DBUG_PRINT("info",("default result: %*s",
                            s->length(), s->c_ptr_quick()));
         CREATE_ON_CALLERS_ARENA(it= new(reuse, &rsize)
-                              Item_string(thd->strmake(s->ptr(),
-                                          s->length()), s->length(),
-                                          it->collation.collation),
-                          use_callers_arena, &backup_current_arena);
+                                Item_string(it->collation.collation),
+                                use_callers_arena, &backup_current_arena);
+        /*
+          We have to use special constructor and allocate string
+          on system heap here. This is because usual Item_string
+          constructor would allocate memory in the callers arena.
+          This would lead to the memory leak in SP loops.
+          See Bug #11333 "Stored Procedure: Memory blow up on
+          repeated SELECT ... INTO query" for sample of such SP.
+          TODO: Usage of the system heap gives significant overhead,
+          however usual "reuse" mechanism does not work here, as
+          Item_string has no max size. That is, if we have a loop, which
+          has string variable with constantly increasing size, we would have
+          to allocate new pieces of memory again and again on each iteration.
+          In future we should probably reserve some area of memory for
+          not-very-large strings and reuse it. But for large strings
+          we would have to use system heap anyway.
+        */
+        ((Item_string*) it)->set_str_with_copy(s->ptr(), s->length());
       }
       break;
     }
Thread
bk commit into 5.0 tree (petr:1.2000) BUG#11333Petr Chardin25 Aug