From: Alexander Nozdrin
Date: May 6 2011 1:39pm
Subject: bzr push into mysql-5.5 branch (alexander.nozdrin:3497 to 3498) Bug#12374486
List-Archive: http://lists.mysql.com/commits/136847
X-Bug: 12374486
Message-Id: <201105061339.p46DdoHF003950@acsmt357.oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

 3498 Alexander Nozdrin	2011-05-06
      Patch for Bug#12374486 - SEVERE MEMORY LEAK IN PREPARED STATEMENTS
      THAT CALL STORED PROCEDURES.
      
      The bug was introduced by WL#4435. The problem was that if a stored
      procedure generated a few result sets with different set of columns,
      a new memory would be allocated after every EXECUTE for every
      result set.
      
      The fix is to introduce a new memory root in scope of MYSQL_STMT,
      and to store result-set metadata in that memory root.

    modified:
      include/mysql.h
      include/mysql.h.pp
      libmysql/libmysql.c
 3497 Alexander Nozdrin	2011-05-06
      Patch for Bug#11848763 / 60025
      (SUBSTRING inside a stored function works too slow).
      
      The user-visible problem was that the server started to consume memory if a
      stored-routine of some sort is executed subsequently. The memory was freed
      only after the corresponding connection was closed.
      
      Technically, the problem was that the memory needed for temporary string
      conversions was allocated on the connection ("persistent") memory root,
      instead of statement one.
      
      The root cause of this problem was the incorrect patch for Bug 55744.
      That patch wrongly fixed a crash in prepared-statement-mode introduced by
      another patch. The patch for Bug 55744 used wrong condition to check if
      prepared statement mode is active (or whether the connection-scoped or
      statement-scoped memory root should be used). The thing is that for
      prepared statements such conversions should be done in the connection
      memory root, so that that the transformations of item-tree were correctly
      remembered in the PREPARE-phase.
      
      The fix is to use proper condition to detect prepared-statement-mode and
      use proper memory root.

    modified:
      sql/item.cc
=== modified file 'include/mysql.h'
--- a/include/mysql.h	2010-11-20 22:56:09 +0000
+++ b/include/mysql.h	2011-05-06 13:39:20 +0000
@@ -573,6 +573,8 @@ typedef struct st_mysql_bind
 } MYSQL_BIND;
 
 
+struct st_mysql_stmt_extension;
+
 /* statement handler */
 typedef struct st_mysql_stmt
 {
@@ -618,7 +620,7 @@ typedef struct st_mysql_stmt
     metadata fields when doing mysql_stmt_store_result.
   */
   my_bool       update_max_length;     
-  void *extension;
+  struct st_mysql_stmt_extension *extension;
 } MYSQL_STMT;
 
 enum enum_stmt_attr_type

=== modified file 'include/mysql.h.pp'
--- a/include/mysql.h.pp	2011-02-11 14:00:09 +0000
+++ b/include/mysql.h.pp	2011-05-06 13:39:20 +0000
@@ -512,6 +512,7 @@ typedef struct st_mysql_bind
   my_bool is_null_value;
   void *extension;
 } MYSQL_BIND;
+struct st_mysql_stmt_extension;
 typedef struct st_mysql_stmt
 {
   MEM_ROOT mem_root;
@@ -541,7 +542,7 @@ typedef struct st_mysql_stmt
   unsigned char bind_result_done;
   my_bool unbuffered_fetch_cancelled;
   my_bool update_max_length;
-  void *extension;
+  struct st_mysql_stmt_extension *extension;
 } MYSQL_STMT;
 enum enum_stmt_attr_type
 {

=== modified file 'libmysql/libmysql.c'
--- a/libmysql/libmysql.c	2011-03-08 17:39:25 +0000
+++ b/libmysql/libmysql.c	2011-05-06 13:39:20 +0000
@@ -94,6 +94,11 @@ sig_handler my_pipe_sig_handler(int sig)
 static my_bool mysql_client_init= 0;
 static my_bool org_my_init_done= 0;
 
+typedef struct st_mysql_stmt_extension
+{
+  MEM_ROOT fields_mem_root;
+} MYSQL_STMT_EXT;
+
 
 /*
   Initialize the MySQL client library
@@ -1480,11 +1485,16 @@ mysql_stmt_init(MYSQL *mysql)
   MYSQL_STMT *stmt;
   DBUG_ENTER("mysql_stmt_init");
 
-  if (!(stmt= (MYSQL_STMT *) my_malloc(sizeof(MYSQL_STMT),
+  if (!(stmt=
+          (MYSQL_STMT *) my_malloc(sizeof (MYSQL_STMT),
+                                   MYF(MY_WME | MY_ZEROFILL))) ||
+      !(stmt->extension=
+          (MYSQL_STMT_EXT *) my_malloc(sizeof (MYSQL_STMT_EXT),
                                        MYF(MY_WME | MY_ZEROFILL))))
   {
     set_mysql_error(mysql, CR_OUT_OF_MEMORY, unknown_sqlstate);
-    DBUG_RETURN(0);
+    my_free(stmt);
+    DBUG_RETURN(NULL);
   }
 
   init_alloc_root(&stmt->mem_root, 2048, 2048);
@@ -1499,6 +1509,8 @@ mysql_stmt_init(MYSQL *mysql)
   strmov(stmt->sqlstate, not_error_sqlstate);
   /* The rest of statement members was bzeroed inside malloc */
 
+  init_alloc_root(&stmt->extension->fields_mem_root, 2048, 0);
+
   DBUG_RETURN(stmt);
 }
 
@@ -1571,6 +1583,7 @@ mysql_stmt_prepare(MYSQL_STMT *stmt, con
     stmt->bind_param_done= stmt->bind_result_done= FALSE;
     stmt->param_count= stmt->field_count= 0;
     free_root(&stmt->mem_root, MYF(MY_KEEP_PREALLOC));
+    free_root(&stmt->extension->fields_mem_root, MYF(0));
 
     int4store(buff, stmt->stmt_id);
 
@@ -1631,21 +1644,21 @@ mysql_stmt_prepare(MYSQL_STMT *stmt, con
 static void alloc_stmt_fields(MYSQL_STMT *stmt)
 {
   MYSQL_FIELD *fields, *field, *end;
-  MEM_ROOT *alloc= &stmt->mem_root;
+  MEM_ROOT *fields_mem_root= &stmt->extension->fields_mem_root;
   MYSQL *mysql= stmt->mysql;
 
-  DBUG_ASSERT(mysql->field_count);
+  DBUG_ASSERT(stmt->field_count);
 
-  stmt->field_count= mysql->field_count;
+  free_root(fields_mem_root, MYF(0));
 
   /*
     Get the field information for non-select statements
     like SHOW and DESCRIBE commands
   */
-  if (!(stmt->fields= (MYSQL_FIELD *) alloc_root(alloc,
+  if (!(stmt->fields= (MYSQL_FIELD *) alloc_root(fields_mem_root,
 						 sizeof(MYSQL_FIELD) *
 						 stmt->field_count)) ||
-      !(stmt->bind= (MYSQL_BIND *) alloc_root(alloc,
+      !(stmt->bind= (MYSQL_BIND *) alloc_root(fields_mem_root,
 					      sizeof(MYSQL_BIND) *
 					      stmt->field_count)))
   {
@@ -1658,18 +1671,36 @@ static void alloc_stmt_fields(MYSQL_STMT
        field && fields < end; fields++, field++)
   {
     *field= *fields; /* To copy all numeric parts. */
-    field->catalog=   strmake_root(alloc, fields->catalog,
+    field->catalog=   strmake_root(fields_mem_root,
+                                   fields->catalog,
                                    fields->catalog_length);
-    field->db=        strmake_root(alloc, fields->db, fields->db_length);
-    field->table=     strmake_root(alloc, fields->table, fields->table_length);
-    field->org_table= strmake_root(alloc, fields->org_table,
+    field->db=        strmake_root(fields_mem_root,
+                                   fields->db,
+                                   fields->db_length);
+    field->table=     strmake_root(fields_mem_root,
+                                   fields->table,
+                                   fields->table_length);
+    field->org_table= strmake_root(fields_mem_root,
+                                   fields->org_table,
                                    fields->org_table_length);
-    field->name=      strmake_root(alloc, fields->name, fields->name_length);
-    field->org_name=  strmake_root(alloc, fields->org_name,
+    field->name=      strmake_root(fields_mem_root,
+                                   fields->name,
+                                   fields->name_length);
+    field->org_name=  strmake_root(fields_mem_root,
+                                   fields->org_name,
                                    fields->org_name_length);
-    field->def=       fields->def ? strmake_root(alloc, fields->def,
-                                                 fields->def_length) : 0;
-    field->def_length= field->def ? fields->def_length : 0;
+    if (fields->def)
+    {
+      field->def= strmake_root(fields_mem_root,
+                               fields->def,
+                               fields->def_length);
+      field->def_length= fields->def_length;
+    }
+    else
+    {
+      field->def= NULL;
+      field->def_length= 0;
+    }
     field->extension= 0; /* Avoid dangling links. */
     field->max_length= 0; /* max_length is set in mysql_stmt_store_result() */
   }
@@ -2387,6 +2418,9 @@ static void reinit_result_set_metadata(M
       prepared statements can't send result set metadata for these queries
       on prepare stage. Read it now.
     */
+
+    stmt->field_count= stmt->mysql->field_count;
+
     alloc_stmt_fields(stmt);
   }
   else
@@ -2404,7 +2438,7 @@ static void reinit_result_set_metadata(M
       previous branch always works.
       TODO: send metadata only when it's really necessary and add a warning
       'Metadata changed' when it's sent twice.
-      */
+    */
     update_stmt_fields(stmt);
   }
 }
@@ -4605,6 +4639,7 @@ my_bool STDCALL mysql_stmt_close(MYSQL_S
 
   free_root(&stmt->result.alloc, MYF(0));
   free_root(&stmt->mem_root, MYF(0));
+  free_root(&stmt->extension->fields_mem_root, MYF(0));
 
   if (mysql)
   {
@@ -4639,6 +4674,7 @@ my_bool STDCALL mysql_stmt_close(MYSQL_S
     }
   }
 
+  my_free(stmt->extension);
   my_free(stmt);
 
   DBUG_RETURN(test(rc));
@@ -4805,16 +4841,13 @@ int STDCALL mysql_stmt_next_result(MYSQL
 
   stmt->state= MYSQL_STMT_EXECUTE_DONE;
   stmt->bind_result_done= FALSE;
+  stmt->field_count= mysql->field_count;
 
   if (mysql->field_count)
   {
     alloc_stmt_fields(stmt);
     prepare_to_fetch_result(stmt);
   }
-  else
-  {
-    stmt->field_count= mysql->field_count;
-  }
 
   DBUG_RETURN(0);
 }

No bundle (reason: useless for push emails).