bk commit into 5.0 tree (pem:1.1913) BUG#10968

[pem]

Below is the list of changes that have just been committed into a local
5.0 repository of pem. When pem 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.1913 05/06/10 16:14:01 pem@stripped +7 -0
  Post review and additional fix for BUG#10968: Stored procedures: crash if long loop.
    Fixed valgrind complaints. This fixes the memory leak problems for
    procedured, and partially for functions. There's still a leak involving
    results from functions that turned out to be too involved, so it will be
    fixed separately.

  sql/sp_rcontext.h
    1.19 05/06/10 16:13:55 pem@stripped +1 -0
    New mem_root pointer used for return fields from functions.

  sql/sp_rcontext.cc
    1.27 05/06/10 16:13:55 pem@stripped +1 -0
    New mem_root pointer used for return fields from functions.

  sql/sp_head.cc
    1.143 05/06/10 16:13:55 pem@stripped +23 -3
    Fixed valgrind problems with the previous memory leak fix (unit cleanup and
    putting result field in a differen mem_root), and removed prealloc flag from
    init_alloc_root() calls.

  sql/item_func.h
    1.118 05/06/10 16:13:55 pem@stripped +2 -8
    Moved Item_func_sp::cleanup() to item_func.cc to ease debugging.

  sql/item_func.cc
    1.211 05/06/10 16:13:55 pem@stripped +12 -1
    Moved Item_func_sp::cleanup() from item_func.h to ease debugging,
    and made a debug output come out right.

  mysql-test/t/sp.test
    1.121 05/06/10 16:13:55 pem@stripped +2 -1
    Fixed some minor mistake (spotted while debugging).

  mysql-test/r/sp.result
    1.127 05/06/10 16:13:55 pem@stripped +2 -6
    Fixed some minor mistake (spotted while debugging).

# 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:	pem
# Host:	mysql.comhem.se
# Root:	/home/pem/work/mysql-5.0

--- 1.210/sql/item_func.cc	Fri Jun  3 13:42:57 2005
+++ 1.211/sql/item_func.cc	Fri Jun 10 16:13:55 2005
@@ -4698,6 +4698,16 @@
   dummy_table= (TABLE*) sql_calloc(sizeof(TABLE));
 }
 
+void
+Item_func_sp::cleanup()
+{
+  if (result_field)
+  {
+    delete result_field;
+    result_field= NULL;
+  }
+  Item_func::cleanup();
+}
 
 const char *
 Item_func_sp::func_name() const
@@ -4746,7 +4756,8 @@
     share->table_cache_key = empty_name;
     share->table_name = empty_name;
   }
-  DBUG_RETURN(m_sp->make_field(max_length, name, dummy_table));
+  field= m_sp->make_field(max_length, name, dummy_table);
+  DBUG_RETURN(field);
 }
 
 

--- 1.117/sql/item_func.h	Thu Jun  2 18:59:36 2005
+++ 1.118/sql/item_func.h	Fri Jun 10 16:13:55 2005
@@ -1308,13 +1308,7 @@
   virtual ~Item_func_sp()
   {}
 
-  void cleanup()
-  {
-    if (result_field)
-      delete result_field;
-    Item_func::cleanup();
-    result_field= NULL;
-  }
+  void cleanup();
 
   const char *func_name() const;
 
@@ -1330,7 +1324,7 @@
   {
     if (execute(&result_field))
       return (longlong) 0;
-    return result_field->val_int();   
+    return result_field->val_int();
   }
 
   double val_real()

--- 1.126/mysql-test/r/sp.result	Wed Jun  1 12:17:38 2005
+++ 1.127/mysql-test/r/sp.result	Fri Jun 10 16:13:55 2005
@@ -251,7 +251,7 @@
 call sub1("sub1b", (select max(i) from t2))|
 call sub1("sub1c", (select i,d from t2 limit 1))|
 call sub1("sub1d", (select 1 from (select 1) a))|
-call sub2("sub2");
+call sub2("sub2")|
 select * from t1|
 id	data
 sub1a	7
@@ -265,6 +265,7 @@
 drop procedure sub1|
 drop procedure sub2|
 drop function sub3|
+delete from t1|
 delete from t2|
 drop procedure if exists a0|
 create procedure a0(x int)
@@ -275,11 +276,6 @@
 call a0(3)|
 select * from t1|
 id	data
-sub1a	7
-sub1b	3
-sub1c	1
-sub1d	1
-sub2	6
 a0	2
 a0	1
 a0	0

--- 1.120/mysql-test/t/sp.test	Wed Jun  1 12:17:38 2005
+++ 1.121/mysql-test/t/sp.test	Fri Jun 10 16:13:55 2005
@@ -372,12 +372,13 @@
 call sub1("sub1b", (select max(i) from t2))|
 call sub1("sub1c", (select i,d from t2 limit 1))|
 call sub1("sub1d", (select 1 from (select 1) a))|
-call sub2("sub2");
+call sub2("sub2")|
 select * from t1|
 select sub3((select max(i) from t2))|
 drop procedure sub1|
 drop procedure sub2|
 drop function sub3|
+delete from t1|
 delete from t2|
 
 # Basic tests of the flow control constructs

--- 1.142/sql/sp_head.cc	Fri Jun  3 19:21:06 2005
+++ 1.143/sql/sp_head.cc	Fri Jun 10 16:13:55 2005
@@ -534,17 +534,35 @@
 }
 
 
+/*
+ *  This is only used for result fields from functions (both during
+ *  fix_length_and_dec() and evaluation).
+ *
+ *  Since the current mem_root during a will be freed and the result
+ *  field will be used by the caller, we have to put it in the caller's
+ *  or main mem_root.
+ */
 Field *
 sp_head::make_field(uint max_length, const char *name, TABLE *dummy)
 {
   Field *field;
+  MEM_ROOT *tmp_mem_root;
+  THD *thd;
   DBUG_ENTER("sp_head::make_field");
+
+  thd= current_thd;
+  tmp_mem_root= thd->mem_root;
+  if (thd->spcont && thd->spcont->callers_mem_root)
+    thd->mem_root= thd->spcont->callers_mem_root;
+  else
+    thd->mem_root= &thd->main_mem_root;
   field= ::make_field((char *)0,
 		!m_returns_len ? max_length : m_returns_len, 
 		(uchar *)"", 0, m_returns_pack, m_returns, m_returns_cs,
 		(enum Field::geometry_type)0, Field::NONE, 
 		m_returns_typelib,
 		name ? name : (const char *)m_name.str, dummy);
+  thd->mem_root= tmp_mem_root;
   DBUG_RETURN(field);
 }
 
@@ -708,7 +726,7 @@
     DBUG_RETURN(-1);
   }
 
-  init_alloc_root(&call_mem_root, MEM_ROOT_BLOCK_SIZE, MEM_ROOT_PREALLOC);
+  init_alloc_root(&call_mem_root, MEM_ROOT_BLOCK_SIZE, 0);
   old_mem_root= thd->mem_root;
   thd->mem_root= &call_mem_root;
   old_free_list= thd->free_list; // Keep the old list
@@ -716,7 +734,8 @@
 
   // QQ Should have some error checking here? (types, etc...)
   nctx= new sp_rcontext(csize, hmax, cmax);
-  for (i= 0 ; i < params && i < argcount ; i++)
+  nctx->callers_mem_root= old_mem_root;
+  for (i= 0 ; i < argcount ; i++)
   {
     sp_pvar_t *pvar = m_pcont->find_pvar(i);
     Item *it= sp_eval_func_item(thd, argp++, pvar->type, NULL);
@@ -812,7 +831,7 @@
     DBUG_RETURN(-1);
   }
 
-  init_alloc_root(&call_mem_root, MEM_ROOT_BLOCK_SIZE, MEM_ROOT_PREALLOC);
+  init_alloc_root(&call_mem_root, MEM_ROOT_BLOCK_SIZE, 0);
   old_mem_root= thd->mem_root;
   thd->mem_root= &call_mem_root;
   old_free_list= thd->free_list; // Keep the old list
@@ -965,6 +984,7 @@
   // Now get rid of the rest of the callee context
   cleanup_items(call_free_list);
   free_items(call_free_list);
+  thd->lex->unit.cleanup();
   free_root(&call_mem_root, MYF(0));
 
   DBUG_RETURN(ret);

--- 1.26/sql/sp_rcontext.cc	Fri May 27 12:03:33 2005
+++ 1.27/sql/sp_rcontext.cc	Fri Jun 10 16:13:55 2005
@@ -32,6 +32,7 @@
   : m_count(0), m_fsize(fsize), m_result(NULL), m_hcount(0), m_hsp(0),
     m_hfound(-1), m_ccount(0)
 {
+  callers_mem_root= NULL;
   in_handler= FALSE;
   m_frame= (Item **)sql_alloc(fsize * sizeof(Item*));
   m_handler= (sp_handler_t *)sql_alloc(hmax * sizeof(sp_handler_t));

--- 1.18/sql/sp_rcontext.h	Fri May 27 12:03:33 2005
+++ 1.19/sql/sp_rcontext.h	Fri Jun 10 16:13:55 2005
@@ -47,6 +47,7 @@
 
  public:
 
+  MEM_ROOT *callers_mem_root;	// Used to store result fields
   bool in_handler;
 
   sp_rcontext(uint fsize, uint hmax, uint cmax);