memory leacks in PS with subqueries fixed (adddition to fix of BUG#2462)

sql/item.h

@@ -419,6 +419,7 @@ public:
   int save_in_field(Field *field, bool no_conversions);
   bool basic_const_item() const { return 1; }
   Item *new_item() { return new Item_int(name,value,max_length); }
+  void cleanup() { fixed= 1; } // to privent drop fixed flag
   void print(String *str);
 };
 
@@ -906,6 +907,8 @@ public:
   enum Type type() const { return CACHE_ITEM; }
   static Item_cache* get_cache(Item_result type);
   table_map used_tables() const { return used_table_map; }
+  virtual void keep_array() {}
+  void cleanup() { fixed= 1; } // to privent drop fixed flag
   void print(String *str);
 };
 
@@ -958,8 +961,10 @@ class Item_cache_row: public Item_cache
 {
   Item_cache  **values;
   uint item_count;
+  bool save_array;
 public:
-  Item_cache_row(): Item_cache(), values(0), item_count(2) {}
+  Item_cache_row()
+    :Item_cache(), values(0), item_count(2), save_array(0) {}
   
   /*
     'allocate' used only in row transformer, to preallocate space for row 
@@ -1000,11 +1005,15 @@ public:
   bool check_cols(uint c);
   bool null_inside();
   void bring_value();
+  void keep_array() { save_array= 1; }
   void cleanup()
   {
     DBUG_ENTER("Item_cache_row::cleanup");
     Item_cache::cleanup();
-    values= 0;
+    if (save_array)
+      bzero(values, item_count*sizeof(Item**));
+    else
+      values= 0;
     DBUG_VOID_RETURN;
   }
 };

sql/item_cmpfunc.cc

@@ -445,7 +445,6 @@ bool Item_in_optimizer::fix_left(THD *thd,
 }
 
 
-
 bool Item_in_optimizer::fix_fields(THD *thd, struct st_table_list *tables,
 				   Item ** ref)
 {
@@ -471,6 +470,7 @@ bool Item_in_optimizer::fix_fields(THD *thd, struct st_table_list *tables,
   return 0;
 }
 
+
 longlong Item_in_optimizer::val_int()
 {
   cache->store(args[0]);
@@ -484,26 +484,38 @@ longlong Item_in_optimizer::val_int()
   return tmp;
 }
 
+
+void Item_in_optimizer::keep_top_level_cache()
+{
+  cache->keep_array();
+  save_cache= 1;
+}
+
+
 void Item_in_optimizer::cleanup()
 {
   DBUG_ENTER("Item_in_optimizer::cleanup");
   Item_bool_func::cleanup();
-  cache= 0;
+  if (!save_cache)
+    cache= 0;
   DBUG_VOID_RETURN;
 }
 
+
 bool Item_in_optimizer::is_null()
 {
   cache->store(args[0]);
   return (null_value= (cache->null_value || args[1]->is_null()));
 }
 
+
 longlong Item_func_eq::val_int()
 {
   int value= cmp.compare();
   return value == 0 ? 1 : 0;
 }
 
+
 /* Same as Item_func_eq, but NULL = NULL */
 
 void Item_func_equal::fix_length_and_dec()

sql/item_cmpfunc.h

@@ -91,9 +91,10 @@ class Item_in_optimizer: public Item_bool_func
 {
 protected:
   Item_cache *cache;
+  bool save_cache;
 public:
   Item_in_optimizer(Item *a, Item_in_subselect *b):
-    Item_bool_func(a, (Item *)b), cache(0) {}
+    Item_bool_func(a, (Item *)b), cache(0), save_cache(0) {}
   bool fix_fields(THD *, struct st_table_list *, Item **);
   bool fix_left(THD *thd, struct st_table_list *tables, Item **ref);
   bool is_null();
@@ -108,6 +109,7 @@ public:
   void cleanup();
   const char *func_name() const { return "<in_optimizer>"; }
   Item_cache **get_cache() { return &cache; }
+  void keep_top_level_cache();
 };
 
 class Comp_creator

sql/item_subselect.cc

@@ -322,14 +322,14 @@ Item_singlerow_subselect::select_transformer(JOIN *join)
 	goto err;
     }
     if (stmt)
-      thd->restore_backup_item_arena(&backup);
+      thd->restore_backup_item_arena(stmt, &backup);
     return RES_REDUCE;
   }
   return RES_OK;
   
 err:
   if (stmt)
-    thd->restore_backup_item_arena(&backup);
+    thd->restore_backup_item_arena(stmt, &backup);
   return RES_ERROR;
 }
 
@@ -789,7 +789,7 @@ Item_in_subselect::single_value_transformer(JOIN *join,
 		       ER_SELECT_REDUCED, warn_buff);
 	}
 	if (stmt)
-	  thd->set_item_arena(&backup);
+	  thd->restore_backup_item_arena(stmt, &backup);
 	DBUG_RETURN(RES_REDUCE);
       }
     }
@@ -797,12 +797,12 @@ Item_in_subselect::single_value_transformer(JOIN *join,
 
 ok:
   if (stmt)
-    thd->restore_backup_item_arena(&backup);
+    thd->restore_backup_item_arena(stmt, &backup);
   DBUG_RETURN(RES_OK);
 
 err:
   if (stmt)
-    thd->restore_backup_item_arena(&backup);
+    thd->restore_backup_item_arena(stmt, &backup);
   DBUG_RETURN(RES_ERROR);
 }
 
@@ -845,6 +845,10 @@ Item_in_subselect::row_value_transformer(JOIN *join)
       thd->lex->current_select= current;
       goto err;
     }
+
+    // we will refer to apper level cache array => we have to save it in PS
+    optimizer->keep_top_level_cache();
+
     thd->lex->current_select= current;
     unit->uncacheable|= UNCACHEABLE_DEPENDENT;
   }
@@ -892,12 +896,12 @@ Item_in_subselect::row_value_transformer(JOIN *join)
       goto err;
   }
   if (stmt)
-    thd->restore_backup_item_arena(&backup);
+    thd->restore_backup_item_arena(stmt, &backup);
   DBUG_RETURN(RES_OK);
 
 err:
   if (stmt)
-    thd->restore_backup_item_arena(&backup);
+    thd->restore_backup_item_arena(stmt, &backup);
   DBUG_RETURN(RES_ERROR);
 }
 
@@ -975,6 +979,7 @@ void subselect_single_select_engine::cleanup()
 {
   DBUG_ENTER("subselect_single_select_engine::cleanup");
   prepared= optimized= executed= 0;
+  join= 0;
   DBUG_VOID_RETURN;
 }
 

sql/mysql_priv.h

@@ -347,6 +347,9 @@ inline THD *_current_thd(void)
 #include "sql_udf.h"
 #include "item.h"
 typedef Comp_creator* (*chooser_compare_func_creator)(bool invert);
+/* sql_parse.cc */
+void free_items(Item *item);
+void cleanup_items(Item *item);
 #include "sql_class.h"
 #include "opt_range.h"
 
@@ -408,7 +411,6 @@ bool mysql_rename_tables(THD *thd, TABLE_LIST *table_list);
 bool mysql_change_db(THD *thd,const char *name);
 void mysql_parse(THD *thd,char *inBuf,uint length);
 bool is_update_query(enum enum_sql_command command);
-void free_items(Item *item);
 bool alloc_query(THD *thd, char *packet, ulong packet_length);
 void mysql_init_select(LEX *lex);
 void mysql_init_query(THD *thd);
@@ -761,9 +763,6 @@ uint find_type(TYPELIB *lib, const char *find, uint length, bool part_match);
 uint check_word(TYPELIB *lib, const char *val, const char *end,
 		const char **end_of_word);
 
-/* sql_parse.cc */
-void free_items(Item *item);
-void cleanup_items(Item *item);
 
 #define MY_DB_OPT_FILE "db.opt"
 bool load_db_opt(THD *thd, const char *path, HA_CREATE_INFO *create);

sql/sql_base.cc

@@ -2039,6 +2039,9 @@ int setup_wild(THD *thd, TABLE_LIST *tables, List<Item> &fields,
 {
   if (!wild_num)
     return 0;
+  Statement *stmt= thd->current_statement, backup;
+  if (stmt)
+    thd->set_n_backup_item_arena(stmt, &backup);
   reg2 Item *item;
   List_iterator<Item> it(fields);
   while ( wild_num && (item= it++))
@@ -2050,7 +2053,11 @@ int setup_wild(THD *thd, TABLE_LIST *tables, List<Item> &fields,
       uint elem= fields.elements;
       if (insert_fields(thd,tables,((Item_field*) item)->db_name,
 			((Item_field*) item)->table_name, &it))
+      {
+	if (stmt)
+	  thd->restore_backup_item_arena(stmt, &backup);
 	return (-1);
+      }
       if (sum_func_list)
       {
 	/*
@@ -2063,6 +2070,8 @@ int setup_wild(THD *thd, TABLE_LIST *tables, List<Item> &fields,
       wild_num--;
     }
   }
+  if (stmt)
+      thd->restore_backup_item_arena(stmt, &backup);
   return 0;
 }
 

sql/sql_class.cc

@@ -1244,22 +1244,28 @@ void Statement::set_statement(Statement *stmt)
   mem_root=       stmt->mem_root;
 }
 
+
 void Statement::set_n_backup_item_arena(Statement *set, Statement *backup)
 {
-  backup->mem_root= mem_root;
+  backup->set_item_arena(this);
-  backup->free_list= free_list;
   set_item_arena(set);
 }
 
 
+void Statement::restore_backup_item_arena(Statement *set, Statement *backup)
+{
+  set->set_item_arena(this);
+  set_item_arena(backup);
+  // reset backup mem_root to avoid its freeing
+  init_alloc_root(&backup->mem_root, 0, 0);
+}
+
 void Statement::set_item_arena(Statement *set)
 {
   mem_root= set->mem_root;
   free_list= set->free_list;
 }
 
-
-
 Statement::~Statement()
 {
   free_root(&mem_root, MYF(0));

sql/sql_class.h

@@ -504,14 +504,31 @@ public:
   /* return class type */
   virtual Type type() const;
 
-  void set_n_backup_item_arena(Statement *set, Statement *backup);
+  inline gptr alloc(unsigned int size) { return alloc_root(&mem_root,size); }
-  inline void restore_backup_item_arena(Statement *backup)
+  inline gptr calloc(unsigned int size)
   {
-    set_item_arena(backup);
+    gptr ptr;
-    // reset backup mem_root to avoid its freeing
+    if ((ptr=alloc_root(&mem_root,size)))
-    init_alloc_root(&backup->mem_root, 0, 0);
+      bzero((char*) ptr,size);
+    return ptr;
   }
-  void set_item_arena(Statement *set);
+  inline char *strdup(const char *str)
+  { return strdup_root(&mem_root,str); }
+  inline char *strmake(const char *str, uint size)
+  { return strmake_root(&mem_root,str,size); }
+  inline char *memdup(const char *str, uint size)
+  { return memdup_root(&mem_root,str,size); }
+  inline char *memdup_w_gap(const char *str, uint size, uint gap)
+  {
+    gptr ptr;
+    if ((ptr=alloc_root(&mem_root,size+gap)))
+      memcpy(ptr,str,size);
+    return ptr;
+  }
+
+  void set_n_backup_item_arena(Statement *set, Statement *backup);
+  void restore_backup_item_arena(Statement *set, Statement *backup);
+  void Statement::set_item_arena(Statement *set);
 };
 
 
@@ -862,34 +879,14 @@ public:
     return 0;
 #endif
   }
-  inline gptr alloc(unsigned int size) { return alloc_root(&mem_root,size); }
-  inline gptr calloc(unsigned int size)
-  {
-    gptr ptr;
-    if ((ptr=alloc_root(&mem_root,size)))
-      bzero((char*) ptr,size);
-    return ptr;
-  }
-  inline char *strdup(const char *str)
-  { return strdup_root(&mem_root,str); }
-  inline char *strmake(const char *str, uint size)
-  { return strmake_root(&mem_root,str,size); }
-  inline char *memdup(const char *str, uint size)
-  { return memdup_root(&mem_root,str,size); }
-  inline char *memdup_w_gap(const char *str, uint size, uint gap)
-  {
-    gptr ptr;
-    if ((ptr=alloc_root(&mem_root,size+gap)))
-      memcpy(ptr,str,size);
-    return ptr;
-  }
-  bool convert_string(LEX_STRING *to, CHARSET_INFO *to_cs,
-		      const char *from, uint from_length,
-		      CHARSET_INFO *from_cs);
   inline gptr trans_alloc(unsigned int size) 
   { 
     return alloc_root(&transaction.mem_root,size);
   }
+
+  bool convert_string(LEX_STRING *to, CHARSET_INFO *to_cs,
+		      const char *from, uint from_length,
+		      CHARSET_INFO *from_cs);
   void add_changed_table(TABLE *table);
   void add_changed_table(const char *key, long key_length);
   CHANGED_TABLE_LIST * changed_table_dup(const char *key, long key_length);
@@ -912,6 +909,32 @@ public:
   }
   inline CHARSET_INFO *charset() { return variables.character_set_client; }
   void update_charset();
+
+  inline void ps_setup_prepare_memory()
+  {
+    DBUG_ASSERT(current_statement!=0);
+    /*
+      We do not want to have in PS memory all that junk,
+      which will be created by preparation => substitute memory
+      from original thread pool.
+
+      We know that PS memory pool is now copied to THD, we move it back
+      to allow some code use it.
+    */
+    current_statement->set_item_arena(this);
+    init_sql_alloc(&mem_root,
+		   variables.query_alloc_block_size,
+		   variables.query_prealloc_size);
+    free_list= 0;
+  }
+  inline void ps_setup_free_memory()
+  {
+    DBUG_ASSERT(current_statement!=0);
+    cleanup_items(current_statement->free_list);
+    free_items(free_list);
+    free_root(&mem_root, MYF(0));
+    set_item_arena(current_statement);
+  }
 };
 
 /* Flags for the THD::system_thread (bitmap) variable */

sql/sql_lex.cc

@@ -1496,11 +1496,17 @@ bool st_select_lex::setup_ref_array(THD *thd, uint order_group_num)
 {
   if (ref_pointer_array)
     return 0;
+
+  /*
+    We have to create array in prepared statement memory if it is
+    prepared statement
+  */
+  Statement *stmt= thd->current_statement ? thd->current_statement : thd;
   return (ref_pointer_array= 
-	  (Item **)thd->alloc(sizeof(Item*) *
+	  (Item **)stmt->alloc(sizeof(Item*) *
-			      (item_list.elements +
+			       (item_list.elements +
-			       select_n_having_items +
+				select_n_having_items +
-			       order_group_num)* 5)) == 0;
+				order_group_num)* 5)) == 0;
 }
 
 

sql/sql_lex.h

@@ -355,6 +355,7 @@ public:
   int prepare(THD *thd, select_result *result, ulong additional_options);
   int exec();
   int cleanup();
+  inline void unclean() { cleaned= 0; }
   void reinit_exec_mechanism();
 
   bool check_updateable(char *db, char *table);

sql/sql_prepare.cc

@@ -630,8 +630,13 @@ static bool mysql_test_insert_fields(Prepared_statement *stmt,
     uint value_count;
     ulong counter= 0;
     
+    thd->ps_setup_prepare_memory();
     if (check_insert_fields(thd,table,fields,*values,1))
+    {
+      thd->ps_setup_free_memory();
       DBUG_RETURN(1);
+    }
+    thd->ps_setup_free_memory();
 
     value_count= values->elements;
     its.rewind();
@@ -679,10 +684,16 @@ static bool mysql_test_upd_fields(Prepared_statement *stmt,
 #endif
   if (open_and_lock_tables(thd, table_list))
     DBUG_RETURN(1);
+
+  thd->ps_setup_prepare_memory();
   if (setup_tables(table_list, 0) ||
-      setup_fields(thd, 0, table_list, fields, 1, 0, 0) || 
+      setup_fields(thd, 0, table_list, fields, 1, 0, 0) ||
-      setup_conds(thd, table_list, &conds) || thd->net.report_error)      
+      setup_conds(thd, table_list, &conds) || thd->net.report_error)
+  {
+    thd->ps_setup_free_memory();
     DBUG_RETURN(1);
+  }
+  thd->ps_setup_free_memory();
 
   /* 
      Currently return only column list info only, and we are not
@@ -753,28 +764,26 @@ static bool mysql_test_select_fields(Prepared_statement *stmt,
     }
 
     thd->used_tables= 0;	// Updated by setup_fields  
-    Statement backup;
+    thd->ps_setup_prepare_memory();
-    /*
+
-      we do not want to have in statement memory all that junk,
+    if (unit->prepare(thd, result, 0))
-      which will be created by preparation => substitute memory
-      from original thread pool
-    */
-    thd->set_n_backup_item_arena(&thd->stmt_backup, &backup);
-    if ((unit->prepare(thd, result, 0)))
     {
-      thd->restore_backup_item_arena(&backup);
+      unit->cleanup();
+      thd->ps_setup_free_memory();
       DBUG_RETURN(1);
     }
-    thd->restore_backup_item_arena(&backup);
    
     if (send_prep_stmt(stmt, fields.elements) ||
         thd->protocol_simple.send_fields(&fields, 0)
 #ifndef EMBEDDED_LIBRARY
-         || net_flush(&thd->net)
+	|| net_flush(&thd->net)
 #endif
-       )
+	)
+    {
       DBUG_RETURN(1);
+    }
     unit->cleanup();
+    thd->ps_setup_free_memory();
   }
   DBUG_RETURN(0);  
 }
@@ -933,7 +942,6 @@ bool mysql_stmt_prepare(THD *thd, char *packet, uint packet_length)
     sl->prep_where= sl->where;
   }
 
-  cleanup_items(thd->free_list);
   stmt->set_statement(thd);
   thd->set_statement(&thd->stmt_backup);
   thd->current_statement= 0;
@@ -953,6 +961,7 @@ init_param_err:
 alloc_query_err:
   /* Statement map deletes statement on erase */
   thd->stmt_map.erase(stmt);
+  thd->current_statement= 0;
   DBUG_RETURN(1);
 insert_stmt_err:
   thd->current_statement= 0;
@@ -1034,15 +1043,17 @@ void mysql_stmt_execute(THD *thd, char *packet)
       tables->table= 0; // safety - nasty init
       tables->table_list= 0;
     }
+
+    sl->master_unit()->unclean();
   }
 
 
 #ifndef EMBEDDED_LIBRARY
   if (stmt->param_count && setup_params_data(stmt))
-    DBUG_VOID_RETURN;
+    goto end;
 #else
   if (stmt->param_count && (*stmt->setup_params_data)(stmt))
-    DBUG_VOID_RETURN;
+    goto end;
 #endif
 
   if (!(specialflag & SPECIAL_NO_PRIOR))
@@ -1065,6 +1076,7 @@ void mysql_stmt_execute(THD *thd, char *packet)
   cleanup_items(stmt->free_list);
   free_root(&thd->mem_root, MYF(0));
   thd->set_statement(&thd->stmt_backup);
+end:
   thd->current_statement= 0;
   DBUG_VOID_RETURN;
 }

tests/client_test.c

@@ -8366,7 +8366,8 @@ int main(int argc, char **argv)
     client_use_result();    /* usage of mysql_use_result() */  
     test_tran_bdb();        /* transaction test on BDB table type */
     test_tran_innodb();     /* transaction test on InnoDB table type */ 
-    test_prepare_ext();     /* test prepare with all types conversion -- TODO */
+    test_prepare_ext();     /* test prepare with all types
+			       conversion -- TODO */
     test_prepare_syntax();  /* syntax check for prepares */
     test_field_names();     /* test for field names */
     test_field_flags();     /* test to help .NET provider team */