A fix and test case for Bug#5315 "mysql_change_user() doesn't free

prepared statements."


include/hash.h:
  New declaration for hash_reset() function. The old version was not used.
libmysql/client_settings.h:
  Declaration for mysql_detach_stmt_list().
libmysql/libmysql.c:
  Fix for bug#5315 "mysql_change_user() doesn't free prepared statements":
  add call to mysql_detach_stmt_list(prepared statements) to
  mysql_change_user(): all statements are freed by server, so client
  counterparts need to be marked as not usable.
mysys/hash.c:
  Fix for bug#5315 "mysql_change_user() doesn't free prepared statements":
  implementation of hash_reset(), which frees all hash elements
  and prepares the hash for reuse.
sql-common/client.c:
  Fix for bug#5315 "mysql_change_user() doesn't free prepared statements":
  implementation of mysql_detach_stmt_list(): zero connection pointer
  in given statement list, thus marking given statements as not usable.
sql/sql_class.cc:
  Fix for bug#5315 "mysql_change_user() doesn't free prepared statements":
  reset prepared statements map in THD::change_user().
sql/sql_class.h:
  Fix for bug#5315 "mysql_change_user() doesn't free prepared statements":
  implementation of Statement_map::reset().
  A little cleanup of ~Statement_map(): first empty names_hash, as st_hash
  has a free function, which will delete statements.
tests/client_test.c:
  A test case for bug #5315 "mysql_change_user() doesn't free prepared
  statements".

include/hash.h

@@ -47,6 +47,7 @@ my_bool _hash_init(HASH *hash, CHARSET_INFO *charset,
 		   uint key_length, hash_get_key get_key,
 		   void (*free_element)(void*), uint flags CALLER_INFO_PROTO);
 void hash_free(HASH *tree);
+void hash_reset(HASH *hash);
 byte *hash_element(HASH *hash,uint idx);
 gptr hash_search(HASH *info,const byte *key,uint length);
 gptr hash_next(HASH *info,const byte *key,uint length);
@@ -56,7 +57,6 @@ my_bool hash_update(HASH *hash,byte *record,byte *old_key,uint old_key_length);
 void hash_replace(HASH *hash, uint idx, byte *new_row);
 my_bool hash_check(HASH *hash);			/* Only in debug library */
 
-#define hash_clear(H) bzero((char*) (H),sizeof(*(H)))
 #define hash_inited(H) ((H)->array.buffer != 0)
 
 #ifdef	__cplusplus

libmysql/client_settings.h

@@ -42,6 +42,7 @@ my_bool handle_local_infile(MYSQL *mysql, const char *net_filename);
 
 void mysql_read_default_options(struct st_mysql_options *options,
 				const char *filename,const char *group);
+void mysql_detach_stmt_list(LIST **stmt_list);
 MYSQL * STDCALL
 cli_mysql_real_connect(MYSQL *mysql,const char *host, const char *user,
 		       const char *passwd, const char *db,

libmysql/libmysql.c

@@ -662,6 +662,7 @@ my_bool	STDCALL mysql_change_user(MYSQL *mysql, const char *user,
 				  const char *passwd, const char *db)
 {
   char buff[512],*end=buff;
+  int rc;
   DBUG_ENTER("mysql_change_user");
 
   if (!user)
@@ -695,18 +696,26 @@ my_bool	STDCALL mysql_change_user(MYSQL *mysql, const char *user,
   /* Write authentication package */
   simple_command(mysql,COM_CHANGE_USER, buff,(ulong) (end-buff),1);
 
-  if ((*mysql->methods->read_change_user_result)(mysql, buff, passwd))
-    DBUG_RETURN(1);
-  /* Free old connect information */
-  my_free(mysql->user,MYF(MY_ALLOW_ZERO_PTR));
-  my_free(mysql->passwd,MYF(MY_ALLOW_ZERO_PTR));
-  my_free(mysql->db,MYF(MY_ALLOW_ZERO_PTR));
-
-  /* alloc new connect information */
-  mysql->user=  my_strdup(user,MYF(MY_WME));
-  mysql->passwd=my_strdup(passwd,MYF(MY_WME));
-  mysql->db=    db ? my_strdup(db,MYF(MY_WME)) : 0;
-  DBUG_RETURN(0);
+  rc= (*mysql->methods->read_change_user_result)(mysql, buff, passwd);
+
+  /*
+    The server will close all statements no matter was the attempt
+    to change user successful or not.
+  */
+  mysql_detach_stmt_list(&mysql->stmts);
+  if (rc == 0)
+  {
+    /* Free old connect information */
+    my_free(mysql->user,MYF(MY_ALLOW_ZERO_PTR));
+    my_free(mysql->passwd,MYF(MY_ALLOW_ZERO_PTR));
+    my_free(mysql->db,MYF(MY_ALLOW_ZERO_PTR));
+
+    /* alloc new connect information */
+    mysql->user=  my_strdup(user,MYF(MY_WME));
+    mysql->passwd=my_strdup(passwd,MYF(MY_WME));
+    mysql->db=    db ? my_strdup(db,MYF(MY_WME)) : 0;
+  }
+  DBUG_RETURN(rc);
 }
 
 #if defined(HAVE_GETPWUID) && defined(NO_GETPWUID_DECL)

mysys/hash.c

@@ -88,6 +88,32 @@ void hash_free(HASH *hash)
   DBUG_VOID_RETURN;
 }
 
+
+/*
+  Delete all elements from the hash (the hash itself is to be reused).
+
+  SYNOPSIS
+    hash_reset()
+      hash   the hash to delete elements of
+*/
+
+void hash_reset(HASH *hash)
+{
+  DBUG_ENTER("hash_reset");
+  if (hash->free)
+  {
+    HASH_LINK *link= dynamic_element(&hash->array, 0, HASH_LINK*);
+    HASH_LINK *end= link + hash->records;
+    for (; link < end; ++link)
+      (*hash->free)(link->data);
+  }
+  reset_dynamic(&hash->array);
+  hash->records= 0;
+  hash->blength= 1;
+  hash->current_record= NO_RECORD;
+  DBUG_VOID_RETURN;
+}
+
 	/* some helper functions */
 
 /*

sql-common/client.c

@@ -2239,6 +2239,32 @@ static void mysql_close_free(MYSQL *mysql)
 }
 
 
+/*
+  Clear connection pointer of every statement: this is necessary
+  to give error on attempt to use a prepared statement of closed
+  connection.
+
+  SYNOPSYS
+    mysql_detach_stmt_list()
+      stmt_list  pointer to mysql->stmts
+*/
+
+void mysql_detach_stmt_list(LIST **stmt_list)
+{
+#ifdef MYSQL_CLIENT
+  /* Reset connection handle in all prepared statements. */
+  LIST *element= *stmt_list;
+  for (; element; element= element->next)
+  {
+    MYSQL_STMT *stmt= (MYSQL_STMT *) element->data;
+    stmt->mysql= 0;
+    /* No need to call list_delete for statement here */
+  }
+  *stmt_list= 0;
+#endif /* MYSQL_CLIENT */
+}
+
+
 void STDCALL mysql_close(MYSQL *mysql)
 {
   DBUG_ENTER("mysql_close");
@@ -2255,20 +2281,7 @@ void STDCALL mysql_close(MYSQL *mysql)
     }
     mysql_close_free_options(mysql);
     mysql_close_free(mysql);
-#ifdef MYSQL_CLIENT
-    if (mysql->stmts)
-    {
-      /* Reset connection handle in all prepared statements. */
-      LIST *element;
-      for (element= mysql->stmts; element; element= element->next)
-      {
-        MYSQL_STMT *stmt= (MYSQL_STMT *) element->data;
-        stmt->mysql= 0;
-        /* No need to call list_delete for statement here */
-      }
-      mysql->stmts= 0;
-    }
-#endif /*MYSQL_CLIENT*/
+    mysql_detach_stmt_list(&mysql->stmts);
 #ifndef TO_BE_DELETED
     /* free/close slave list */
     if (mysql->rpl_pivot)

sql/sql_class.cc

@@ -328,6 +328,7 @@ void THD::change_user(void)
   cleanup();
   cleanup_done= 0;
   init();
+  stmt_map.reset();
   hash_init(&user_vars, &my_charset_bin, USER_VARS_HASH_SIZE, 0, 0,
 	    (hash_get_key) get_var_key,
 	    (hash_free_key) free_user_var, 0);

sql/sql_class.h

@@ -566,7 +566,7 @@ public:
   assignment in Statement::Statement)
   Non-empty statement names are unique too: attempt to insert a new statement
   with duplicate name causes older statement to be deleted
-  
+
   Statements are auto-deleted when they are removed from the map and when the
   map is deleted.
 */
@@ -575,7 +575,7 @@ class Statement_map
 {
 public:
   Statement_map();
-  
+
   int insert(Statement *statement);
 
   Statement *find_by_name(LEX_STRING *name)
@@ -608,11 +608,18 @@ public:
     }
     hash_delete(&st_hash, (byte *) statement);
   }
+  /* Erase all statements (calls Statement destructor) */
+  void reset()
+  {
+    hash_reset(&names_hash);
+    hash_reset(&st_hash);
+    last_found_statement= 0;
+  }
 
   ~Statement_map()
   {
-    hash_free(&st_hash);
     hash_free(&names_hash);
+    hash_free(&st_hash);
   }
 private:
   HASH st_hash;

tests/client_test.c

@@ -10391,6 +10391,34 @@ static void test_bug5194()
 }
 
 
+static void test_bug5315()
+{
+  MYSQL_STMT *stmt;
+  const char *stmt_text;
+  int rc;
+
+  myheader("test_bug5315");
+
+  stmt_text= "SELECT 1";
+  stmt= mysql_stmt_init(mysql);
+  rc= mysql_stmt_prepare(stmt, stmt_text, strlen(stmt_text));
+  DBUG_ASSERT(rc == 0);
+  mysql_change_user(mysql, opt_user, opt_password, current_db);
+  rc= mysql_stmt_execute(stmt);
+  DBUG_ASSERT(rc != 0);
+  if (rc)
+    printf("Got error (as expected):\n%s", mysql_stmt_error(stmt));
+  /* check that connection is OK */
+  mysql_stmt_close(stmt);
+  stmt= mysql_stmt_init(mysql);
+  rc= mysql_stmt_prepare(stmt, stmt_text, strlen(stmt_text));
+  DBUG_ASSERT(rc == 0);
+  rc= mysql_stmt_execute(stmt);
+  DBUG_ASSERT(rc == 0);
+  mysql_stmt_close(stmt);
+}
+
+
 /*
   Read and parse arguments and MySQL options from my.cnf
 */
@@ -10694,6 +10722,8 @@ int main(int argc, char **argv)
     test_bug5399();         /* check that statement id uniquely identifies
                                statement */
     test_bug5194();         /* bulk inserts in prepared mode */
+    test_bug5315();         /* check that mysql_change_user closes all
+                               prepared statements */
     /*
       XXX: PLEASE RUN THIS PROGRAM UNDER VALGRIND AND VERIFY THAT YOUR TEST
       DOESN'T CONTAIN WARNINGS/ERRORS BEFORE YOU PUSH.