Fix Bug#11333 "Stored Procedure: Memory blow up on repeated SELECT ... INTO query"

mysql-test/r/sp.result:
  update result
mysql-test/t/sp.test:
  Add test for Bug #11333 "Stored Procedure: Memory blow up on repeated SELECT ... INTO query"
sql/item.cc:
  we should call destructors for Items before reuse
sql/item.h:
  Add new method and constructor for Item_string.
sql/sp_head.cc:
  String allocation should be done on the system heap for now.

mysql-test/r/sp.result

@@ -3166,4 +3166,23 @@ a
 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;

mysql-test/t/sp.test

@@ -3995,6 +3995,40 @@ select * from t2|
 truncate t2|
 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
 #

sql/item.cc

@@ -303,6 +303,7 @@ void *Item::operator new(size_t size, Item *reuse, uint *rsize)
     if (rsize)
       (*rsize)= reuse->rsize;
     reuse->cleanup();
+    delete reuse;
     TRASH((void *)reuse, size);
     return (void *)reuse;
   }

sql/item.h

@@ -1298,6 +1298,15 @@ class Item_string :public Item
     // 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)
   {
@@ -1309,6 +1318,15 @@ class Item_string :public Item
     // 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();
   longlong val_int();

sql/sp_head.cc

@@ -249,10 +249,25 @@ sp_eval_func_item(THD *thd, Item **it_addr, enum enum_field_types type,
         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;
     }