Fixed on BUG#6048: Stored procedure causes operating system reboot

Memory leak in locally evalutated expressions during SP execution fixed by
  reusing allocated item slots when possible.
  Note: No test case added, since the test is a stress test that tries to make
  the machine to run out of memory.
  Second attempt, now tested with debug build, valgrind build, max (optimized)
  build, with and without --debug, --vagrind and --ps-protocol.
  Errors in trigger and view test with --debug in debug build where present
  before this patch, and likewise for valgrind warnings for view test in
  valgrind build with --ps-protocol.

sql/item.cc

@@ -298,7 +298,7 @@ longlong Item::val_int_from_decimal()
 
 
 Item::Item():
-  name(0), orig_name(0), name_length(0), fixed(0),
+  rsize(0), name(0), orig_name(0), name_length(0), fixed(0),
   collation(&my_charset_bin, DERIVATION_COERCIBLE)
 {
   marker= 0;
@@ -330,6 +330,7 @@ Item::Item():
   tables
 */
 Item::Item(THD *thd, Item *item):
+  rsize(0),
   str_value(item->str_value),
   name(item->name),
   orig_name(item->orig_name),

sql/item.h

@@ -232,6 +232,21 @@ public:
   static void *operator new(size_t size) {return (void*) sql_alloc((uint) size); }
   static void *operator new(size_t size, MEM_ROOT *mem_root)
   { return (void*) alloc_root(mem_root, (uint) size); }
+  /* Special for SP local variable assignment - reusing slots */
+  static void *operator new(size_t size, Item *reuse, uint *rsize)
+  {
+    if (reuse && size <= reuse->rsize)
+    {
+      reuse->cleanup();
+      TRASH((void *)reuse, size);
+      if (rsize)
+	(*rsize)= reuse->rsize;
+      return (void *)reuse;
+    }
+    if (rsize)
+      (*rsize)= size;
+    return (void *)sql_alloc((uint)size);
+  }
   static void operator delete(void *ptr,size_t size) { TRASH(ptr, size); }
   static void operator delete(void *ptr, MEM_ROOT *mem_root) {}
 
@@ -247,6 +262,9 @@ public:
 
   enum traverse_order { POSTFIX, PREFIX };
   
+  /* Reuse size, only used by SP local variable assignment, otherwize 0 */
+  uint rsize;
+
   /*
     str_values's main purpose is to be used to cache the value in
     save_in_field

sql/sp_head.cc

@@ -131,10 +131,12 @@ sp_prepare_func_item(THD* thd, Item **it_addr)
 ** if nothing else.
 */
 Item *
-sp_eval_func_item(THD *thd, Item **it_addr, enum enum_field_types type)
+sp_eval_func_item(THD *thd, Item **it_addr, enum enum_field_types type,
+		  Item *reuse)
 {
   DBUG_ENTER("sp_eval_func_item");
   Item *it= sp_prepare_func_item(thd, it_addr);
+  uint rsize;
   DBUG_PRINT("info", ("type: %d", type));
 
   if (!it)
@@ -144,7 +146,7 @@ sp_eval_func_item(THD *thd, Item **it_addr, enum enum_field_types type)
 
   /* QQ How do we do this? Is there some better way? */
   if (type == MYSQL_TYPE_NULL)
-    it= new Item_null();
+    it= new(reuse, &rsize) Item_null();
   else
   {
     switch (sp_map_result_type(type)) {
@@ -155,12 +157,12 @@ sp_eval_func_item(THD *thd, Item **it_addr, enum enum_field_types type)
 	if (it->null_value)
 	{
 	  DBUG_PRINT("info", ("INT_RESULT: null"));
-	  it= new Item_null();
+	  it= new(reuse, &rsize) Item_null();
 	}
 	else
 	{
 	  DBUG_PRINT("info", ("INT_RESULT: %d", i));
-          it= new Item_int(i);
+          it= new(reuse, &rsize) Item_int(i);
 	}
 	break;
       }
@@ -171,7 +173,7 @@ sp_eval_func_item(THD *thd, Item **it_addr, enum enum_field_types type)
 	if (it->null_value)
 	{
 	  DBUG_PRINT("info", ("REAL_RESULT: null"));
-	  it= new Item_null();
+	  it= new(reuse, &rsize) Item_null();
 	}
 	else
 	{
@@ -180,7 +182,7 @@ sp_eval_func_item(THD *thd, Item **it_addr, enum enum_field_types type)
 	  uint8 decimals= it->decimals;
 	  uint32 max_length= it->max_length;
 	  DBUG_PRINT("info", ("REAL_RESULT: %g", d));
-          it= new Item_float(d);
+          it= new(reuse, &rsize) Item_float(d);
 	  it->decimals= decimals;
 	  it->max_length= max_length;
 	}
@@ -190,9 +192,9 @@ sp_eval_func_item(THD *thd, Item **it_addr, enum enum_field_types type)
       {
         my_decimal value, *val= it->val_decimal(&value);
         if (it->null_value)
-          it= new Item_null();
+          it= new(reuse, &rsize) Item_null();
         else
-          it= new Item_decimal(val);
+          it= new(reuse, &rsize) Item_decimal(val);
 #ifndef DBUG_OFF
         char dbug_buff[DECIMAL_MAX_STR_LENGTH+1];
         DBUG_PRINT("info", ("DECIMAL_RESULT: %s", dbug_decimal_as_string(dbug_buff, val)));
@@ -208,14 +210,16 @@ sp_eval_func_item(THD *thd, Item **it_addr, enum enum_field_types type)
 	if (it->null_value)
 	{
 	  DBUG_PRINT("info", ("default result: null"));
-	  it= new Item_null();
+	  it= new(reuse, &rsize) Item_null();
 	}
 	else
 	{
 	  DBUG_PRINT("info",("default result: %*s",
                              s->length(), s->c_ptr_quick()));
-	  it= new Item_string(thd->strmake(s->ptr(), s->length()),
+	  it= new(reuse, &rsize) Item_string(thd->strmake(s->ptr(),
-			      s->length(), it->collation.collation);
+							  s->length()),
+					     s->length(),
+					     it->collation.collation);
 	}
 	break;
       }
@@ -224,6 +228,7 @@ sp_eval_func_item(THD *thd, Item **it_addr, enum enum_field_types type)
       DBUG_ASSERT(0);
     }
   }
+  it->rsize= rsize;
 
   DBUG_RETURN(it);
 }
@@ -708,7 +713,7 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp)
   for (i= 0 ; i < params && i < argcount ; i++)
   {
     sp_pvar_t *pvar = m_pcont->find_pvar(i);
-    Item *it= sp_eval_func_item(thd, argp++, pvar->type);
+    Item *it= sp_eval_func_item(thd, argp++, pvar->type, NULL);
 
     if (it)
       nctx->push_item(it);
@@ -823,7 +828,7 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
 	}
 	else
 	{
-	  Item *it2= sp_eval_func_item(thd, li.ref(), pvar->type);
+	  Item *it2= sp_eval_func_item(thd, li.ref(), pvar->type, NULL);
 
 	  if (it2)
 	    nctx->push_item(it2); // IN or INOUT
@@ -1466,19 +1471,9 @@ sp_instr_set::execute(THD *thd, uint *nextp)
 int
 sp_instr_set::exec_core(THD *thd, uint *nextp)
 {
-  Item *it;
+  int res= thd->spcont->set_item_eval(thd, m_offset, &m_value, m_type);
-  int res;
 
-  it= sp_eval_func_item(thd, &m_value, m_type);
-  if (! it)
-    res= -1;
-  else
-  {
-    res= 0;
-    thd->spcont->set_item(m_offset, it);
-  }
   *nextp = m_ip+1;
-
   return res;
 }
 
@@ -1715,7 +1710,7 @@ sp_instr_freturn::exec_core(THD *thd, uint *nextp)
   Item *it;
   int res;
 
-  it= sp_eval_func_item(thd, &m_value, m_type);
+  it= sp_eval_func_item(thd, &m_value, m_type, NULL);
   if (! it)
     res= -1;
   else

sql/sp_rcontext.cc

@@ -40,19 +40,39 @@ sp_rcontext::sp_rcontext(uint fsize, uint hmax, uint cmax)
   m_saved.empty();
 }
 
-int
-sp_rcontext::set_item_eval(uint idx, Item **item_addr, enum_field_types type)
-{
-  extern Item *sp_eval_func_item(THD *thd, Item **it, enum_field_types type);
-  Item *it= sp_eval_func_item(current_thd, item_addr, type);
 
+int
+sp_rcontext::set_item_eval(THD *thd, uint idx, Item **item_addr,
+			   enum_field_types type)
+{
+  extern Item *sp_eval_func_item(THD *thd, Item **it, enum_field_types type,
+				 Item *reuse);
+  Item *it;
+  Item *reuse_it;
+  Item *old_item_next;
+  Item *old_free_list= thd->free_list;
+  int res;
+  LINT_INIT(old_item_next);
+
+  if ((reuse_it= get_item(idx)))
+    old_item_next= reuse_it->next;
+  it= sp_eval_func_item(thd, item_addr, type, reuse_it);
   if (! it)
-    return -1;
+    res= -1;
   else
   {
+    res= 0;
+    if (reuse_it && it == reuse_it)
+    {
+      // A reused item slot, where the constructor put it in the free_list,
+      // so we have to restore the list.
+      thd->free_list= old_free_list;
+      it->next= old_item_next;
+    }
     set_item(idx, it);
-    return 0;
   }
+
+  return res;
 }
 
 bool
@@ -111,7 +131,10 @@ void
 sp_rcontext::save_variables(uint fp)
 {
   while (fp < m_count)
-    m_saved.push_front(m_frame[fp++]);
+  {
+    m_saved.push_front(m_frame[fp]);
+    m_frame[fp++]= NULL;	// Prevent reuse
+  }
 }
 
 void
@@ -230,7 +253,12 @@ sp_cursor::fetch(THD *thd, List<struct sp_pvar> *vars)
   for (fldcount= 0 ; (pv= li++) ; fldcount++)
   {
     Item *it;
+    Item *reuse;
+    uint rsize;
+    Item *old_item_next;
+    Item *old_free_list= thd->free_list;
     const char *s;
+    LINT_INIT(old_item_next);
 
     if (fldcount >= m_prot->get_field_count())
     {
@@ -238,9 +266,13 @@ sp_cursor::fetch(THD *thd, List<struct sp_pvar> *vars)
                  ER(ER_SP_WRONG_NO_OF_FETCH_ARGS), MYF(0));
       return -1;
     }
+
+    if ((reuse= thd->spcont->get_item(pv->offset)))
+      old_item_next= reuse->next;
+
     s= row[fldcount];
     if (!s)
-      it= new Item_null();
+      it= new(reuse, &rsize) Item_null();
     else
     {
       /*
@@ -255,23 +287,32 @@ sp_cursor::fetch(THD *thd, List<struct sp_pvar> *vars)
       len= (*next -s)-1;
       switch (sp_map_result_type(pv->type)) {
       case INT_RESULT:
-	it= new Item_int(s);
+	it= new(reuse, &rsize) Item_int(s);
 	break;
       case REAL_RESULT:
-        it= new Item_float(s, len);
+        it= new(reuse, &rsize) Item_float(s, len);
 	break;
       case DECIMAL_RESULT:
-        it= new Item_decimal(s, len, thd->db_charset);
+        it= new(reuse, &rsize) Item_decimal(s, len, thd->db_charset);
         break;
       case STRING_RESULT:
         /* TODO: Document why we do an extra copy of the string 's' here */
-        it= new Item_string(thd->strmake(s, len), len, thd->db_charset);
+        it= new(reuse, &rsize) Item_string(thd->strmake(s, len), len,
+					   thd->db_charset);
         break;
       case ROW_RESULT:
       default:
         DBUG_ASSERT(0);
       }
     }
+    it->rsize= rsize;
+    if (reuse && it == reuse)
+    {
+      // A reused item slot, where the constructor put it in the free_list,
+      // so we have to restore the list.
+      thd->free_list= old_free_list;
+      it->next= old_item_next;
+    }
     thd->spcont->set_item(pv->offset, it);
   }
   if (fldcount < m_prot->get_field_count())

sql/sp_rcontext.h

@@ -62,19 +62,19 @@ class sp_rcontext : public Sql_alloc
   push_item(Item *i)
   {
     if (m_count < m_fsize)
-      m_frame[m_count++] = i;
+      m_frame[m_count++]= i;
   }
 
   inline void
   set_item(uint idx, Item *i)
   {
     if (idx < m_count)
-      m_frame[idx] = i;
+      m_frame[idx]= i;
   }
 
   /* Returns 0 on success, -1 on (eval) failure */
   int
-  set_item_eval(uint idx, Item **i, enum_field_types type);
+  set_item_eval(THD *thd, uint idx, Item **i, enum_field_types type);
 
   inline Item *
   get_item(uint idx)
@@ -82,7 +82,6 @@ class sp_rcontext : public Sql_alloc
     return m_frame[idx];
   }
 
-
   inline Item **
   get_item_addr(uint idx)
   {

sql/sql_class.cc

@@ -1746,7 +1746,8 @@ bool select_dumpvar::send_data(List<Item> &items)
     {
       if ((yy=var_li++)) 
       {
-	if (thd->spcont->set_item_eval(yy->get_offset(), it.ref(), zz->type))
+	if (thd->spcont->set_item_eval(current_thd,
+				       yy->get_offset(), it.ref(), zz->type))
 	  DBUG_RETURN(1);
       }
     }