Commit 03949f8c authored by unknown's avatar unknown
Browse files

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.


mysql-test/r/sp.result:
  Fixed some minor mistake (spotted while debugging).
mysql-test/t/sp.test:
  Fixed some minor mistake (spotted while debugging).
sql/item_func.cc:
  Moved Item_func_sp::cleanup() from item_func.h to ease debugging,
  and made a debug output come out right.
sql/item_func.h:
  Moved Item_func_sp::cleanup() to item_func.cc to ease debugging.
sql/sp_head.cc:
  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/sp_rcontext.cc:
  New mem_root pointer used for return fields from functions.
sql/sp_rcontext.h:
  New mem_root pointer used for return fields from functions.
parent 8331a7cd

mysql-test/r/sp.result

+2 −6
Original line number Diff line number Diff line
@@ -251,7 +251,7 @@ call sub1("sub1a", (select 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 @@ sub3((select max(i) from t2)) 
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 @@ end while| 
call a0(3)|

select * from t1|

id	data

sub1a	7

sub1b	3

sub1c	1

sub1d	1

sub2	6

a0	2

a0	1

a0	0

mysql-test/t/sp.test

+2 −1
Original line number Diff line number Diff line
@@ -372,12 +372,13 @@ call sub1("sub1a", (select 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|

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

sql/item_func.cc

+12 −1
Original line number Diff line number Diff line
@@ -4698,6 +4698,16 @@ Item_func_sp::Item_func_sp(sp_name *name, List<Item> &list) 
  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 @@ Item_func_sp::sp_result_field(void) const 
    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);

}

sql/item_func.h

+2 −8
Original line number Diff line number Diff line
@@ -1308,13 +1308,7 @@ class Item_func_sp :public Item_func 
  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;

sql/sp_head.cc

+23 −3
Original line number Diff line number Diff line
@@ -534,17 +534,35 @@ sp_head::destroy() 
}





/*

 *  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 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp) 
    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 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp) 


  // 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 @@ sp_head::execute_procedure(THD *thd, List<Item> *args) 
    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 @@ sp_head::execute_procedure(THD *thd, List<Item> *args) 
  // 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);