Cleanups during review of new code

7 files changed, 250 insertions, 194 deletions

diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test
index 2cd8ccda90c..877be9607a9 100644
--- a/mysql-test/t/sp.test
+++ b/mysql-test/t/sp.test
@@ -2468,25 +2468,26 @@ drop table t3|
 #
 # BUG#4318
 #
-#QQ Don't know if HANDLER commands can work with SPs, or at all...
-# 
-#create table t3 (s1 int)|
-#insert into t3 values (3), (4)|
-#
-#--disable_warnings
-#drop procedure if exists bug4318|
-#--enable_warnings
-#create procedure bug4318()
-#  handler t3 read next|
-#
-#handler t3 open|
-## Expect no results, as tables are closed, but there shouldn't be any errors
-#call bug4318()|
-#call bug4318()|
-#handler t3 close|
-#
-#drop procedure bug4318|
-#drop table t3|
+
+--disable_parsing Don't know if HANDLER commands can work with SPs, or at all..
+create table t3 (s1 int)|
+insert into t3 values (3), (4)|
+
+--disable_warnings
+drop procedure if exists bug4318|
+--enable_warnings
+create procedure bug4318()
+  handler t3 read next|
+
+handler t3 open|
+# Expect no results, as tables are closed, but there shouldn't be any errors
+call bug4318()|
+call bug4318()|
+handler t3 close|
+
+drop procedure bug4318|
+drop table t3|
+--enable_parsing
 
 #
 # BUG#4902: Stored procedure with SHOW WARNINGS leads to packet error
@@ -2834,26 +2835,27 @@ drop table t3|
 #
 # BUG#6022: Stored procedure shutdown problem with self-calling function.
 #
-# This part of test is disabled until we implement support for
-# recursive stored functions.
-#--disable_warnings
-#drop function if exists bug6022|
-#--enable_warnings
-#
-#--disable_warnings
-#drop function if exists bug6022|
-#--enable_warnings
-#create function bug6022(x int) returns int
-#begin
-#  if x < 0 then
-#    return 0;
-#  else
-#    return bug6022(x-1);
-#  end if;
-#end|
-#
-#select bug6022(5)|
-#drop function bug6022|
+
+--disable_parsing until we implement support for recursive stored functions.
+--disable_warnings
+drop function if exists bug6022|
+--enable_warnings
+
+--disable_warnings
+drop function if exists bug6022|
+--enable_warnings
+create function bug6022(x int) returns int
+begin
+  if x < 0 then
+    return 0;
+  else
+    return bug6022(x-1);
+  end if;
+end|
+
+select bug6022(5)|
+drop function bug6022|
+--enable_parsing
 
 #
 # BUG#6029: Stored procedure specific handlers should have priority
@@ -3760,27 +3762,28 @@ drop procedure if exists bug7088_1|
 drop procedure if exists bug7088_2|
 --enable_warnings
 
-# psergey: temporarily disabled until Bar fixes BUG#11986
-# create procedure bug6063()
-#   lābel: begin end|
-# call bug6063()|
-# # QQ Known bug: this will not show the label correctly.
-# show create procedure bug6063|
-# 
-# set character set utf8|
-# create procedure bug7088_1()
-#   label1: begin end label1|
-# create procedure bug7088_2()
-#   lƤbel1: begin end|
-# call bug7088_1()|
-# call bug7088_2()|
-# set character set default|
-# show create procedure bug7088_1|
-# show create procedure bug7088_2|
-# 
-# drop procedure bug6063|
-# drop procedure bug7088_1|
-# drop procedure bug7088_2|
+--disable_parsing temporarily disabled until Bar fixes BUG#11986
+create procedure bug6063()
+  lābel: begin end|
+call bug6063()|
+# QQ Known bug: this will not show the label correctly.
+show create procedure bug6063|
+
+set character set utf8|
+create procedure bug7088_1()
+  label1: begin end label1|
+create procedure bug7088_2()
+  lƤbel1: begin end|
+call bug7088_1()|
+call bug7088_2()|
+set character set default|
+show create procedure bug7088_1|
+show create procedure bug7088_2|
+
+drop procedure bug6063|
+drop procedure bug7088_1|
+drop procedure bug7088_2|
+--enable_parsing
 
 #
 # BUG#9565: "Wrong locking in stored procedure if a sub-sequent procedure
diff --git a/sql/lock.cc b/sql/lock.cc
index 568ca2b68af..941d7baa76e 100644
--- a/sql/lock.cc
+++ b/sql/lock.cc
@@ -849,10 +849,6 @@ static void print_lock_error(int error, const char *table)
   So in this exceptional case the COMMIT should not be blocked by the FLUSH
   TABLES WITH READ LOCK.
 
-  TODO in MySQL 5.x: make_global_read_lock_block_commit() should be
-  killable. Normally CPU does not spend a long time in this function (COMMITs
-  are quite fast), but it would still be nice.
-
 ****************************************************************************/
 
 volatile uint global_read_lock=0;
@@ -1003,7 +999,7 @@ bool make_global_read_lock_block_commit(THD *thd)
     pthread_cond_wait(&COND_refresh, &LOCK_global_read_lock);
   DBUG_EXECUTE_IF("make_global_read_lock_block_commit_loop",
                   protect_against_global_read_lock--;);
-  if (error= thd->killed)
+  if ((error= test(thd->killed)))
     global_read_lock_blocks_commit--; // undo what we did
   else
     thd->global_read_lock= MADE_GLOBAL_READ_LOCK_BLOCK_COMMIT;
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index f8bfcb75be2..4989b0e623c 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -1908,7 +1908,8 @@ static void check_data_home(const char *path)
 static void sig_reload(int signo)
 {
  // Flush everything
-  reload_acl_and_cache((THD*) 0,REFRESH_LOG, (TABLE_LIST*) 0, NULL);
+  bool not_used;
+  reload_acl_and_cache((THD*) 0,REFRESH_LOG, (TABLE_LIST*) 0, &not_used);
   signal(signo, SIG_ACK);
 }
 
@@ -2267,12 +2268,13 @@ static void *signal_hand(void *arg __attribute__((unused)))
     case SIGHUP:
       if (!abort_loop)
       {
+        bool not_used;
 	mysql_print_status();		// Print some debug info
 	reload_acl_and_cache((THD*) 0,
 			     (REFRESH_LOG | REFRESH_TABLES | REFRESH_FAST |
 			      REFRESH_GRANT |
 			      REFRESH_THREADS | REFRESH_HOSTS),
-			     (TABLE_LIST*) 0, NULL); // Flush logs
+			     (TABLE_LIST*) 0, &not_used); // Flush logs
       }
       break;
 #ifdef USE_ONE_SIGNAL_HAND
diff --git a/sql/sp_cache.cc b/sql/sp_cache.cc
index c8f0ed6ba2d..31cecff6e50 100644
--- a/sql/sp_cache.cc
+++ b/sql/sp_cache.cc
@@ -22,7 +22,7 @@
 #include "sp_head.h"
 
 static pthread_mutex_t Cversion_lock;
-static ulong Cversion = 0;
+static ulong volatile Cversion = 0;
 
 void
 sp_cache_init()
@@ -30,6 +30,7 @@ sp_cache_init()
   pthread_mutex_init(&Cversion_lock, MY_MUTEX_INIT_FAST);
 }
 
+
 void
 sp_cache_clear(sp_cache **cp)
 {
@@ -42,33 +43,31 @@ sp_cache_clear(sp_cache **cp)
   }
 }
 
+
 void
 sp_cache_insert(sp_cache **cp, sp_head *sp)
 {
   sp_cache *c= *cp;
+  ulong v;
 
   if (! c)
-    c= new sp_cache();
-  if (c)
   {
-    ulong v;
-
-    pthread_mutex_lock(&Cversion_lock); // LOCK
-    v= Cversion;
-    pthread_mutex_unlock(&Cversion_lock); // UNLOCK
+    if (!(c= new sp_cache()))
+      return;                                   // End of memory error
+  }
 
-    if (c->version < v)
-    {
-      if (*cp)
-	c->remove_all();
-      c->version= v;
-    }
-    c->insert(sp);
-    if (*cp == NULL)
-      *cp= c;
+  v= Cversion;		/* No need to lock when reading long variable */
+  if (c->version < v)
+  {
+    if (*cp)
+      c->remove_all();
+    c->version= v;
   }
+  c->insert(sp);
+  *cp= c;                                       // Update *cp if it was NULL
 }
 
+
 sp_head *
 sp_cache_lookup(sp_cache **cp, sp_name *name)
 {
@@ -78,10 +77,8 @@ sp_cache_lookup(sp_cache **cp, sp_name *name)
   if (! c)
     return NULL;
 
-  pthread_mutex_lock(&Cversion_lock); // LOCK
-  v= Cversion;
-  pthread_mutex_unlock(&Cversion_lock); // UNLOCK
-
+  
+  v= Cversion;		/* No need to lock when reading long variable */
   if (c->version < v)
   {
     c->remove_all();
@@ -91,6 +88,7 @@ sp_cache_lookup(sp_cache **cp, sp_name *name)
   return c->lookup(name->m_qname.str, name->m_qname.length);
 }
 
+
 bool
 sp_cache_remove(sp_cache **cp, sp_name *name)
 {
@@ -114,14 +112,14 @@ sp_cache_remove(sp_cache **cp, sp_name *name)
   return found;
 }
 
+
 void
 sp_cache_invalidate()
 {
-  pthread_mutex_lock(&Cversion_lock); // LOCK
-  Cversion++;
-  pthread_mutex_unlock(&Cversion_lock); // UNLOCK
+  thread_safe_increment(Cversion, &Cversion_lock); // UNLOCK
 }
 
+
 static byte *
 hash_get_key_for_sp_head(const byte *ptr, uint *plen,
 			       my_bool first)
@@ -132,6 +130,7 @@ hash_get_key_for_sp_head(const byte *ptr, uint *plen,
   return (byte*) sp->m_qname.str;
 }
 
+
 static void
 hash_free_sp_head(void *p)
 {
@@ -140,16 +139,19 @@ hash_free_sp_head(void *p)
   delete sp;
 }
 
+
 sp_cache::sp_cache()
 {
   init();
 }
 
+
 sp_cache::~sp_cache()
 {
   hash_free(&m_hashtable);
 }
 
+
 void
 sp_cache::init()
 {
@@ -158,6 +160,7 @@ sp_cache::init()
   version= 0;
 }
 
+
 void
 sp_cache::cleanup()
 {
diff --git a/sql/sp_head.cc b/sql/sp_head.cc
index 3a386356335..102363b9130 100644
--- a/sql/sp_head.cc
+++ b/sql/sp_head.cc
@@ -678,12 +678,14 @@ sp_head::execute(THD *thd)
       cleanup_items(i->free_list);
     i->state= Query_arena::EXECUTED;
 
-    // Check if an exception has occurred and a handler has been found
-    // Note: We havo to check even if ret==0, since warnings (and some
-    //       errors don't return a non-zero value.
-    //       We also have to check even if thd->killed != 0, since some
-    //       errors return with this even when a handler has been found
-    //       (e.g. "bad data").
+    /*
+      Check if an exception has occurred and a handler has been found
+      Note: We havo to check even if ret==0, since warnings (and some
+      errors don't return a non-zero value.
+      We also have to check even if thd->killed != 0, since some
+      errors return with this even when a handler has been found
+      (e.g. "bad data").
+    */
     if (ctx)
     {
       uint hf;
@@ -759,8 +761,10 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp)
 
   if (argcount != params)
   {
-    // Need to use my_printf_error here, or it will not terminate the
-    // invoking query properly.
+    /*
+      Need to use my_printf_error here, or it will not terminate the
+      invoking query properly.
+    */
     my_error(ER_SP_WRONG_NO_OF_ARGS, MYF(0),
              "FUNCTION", m_qname.str, params, argcount);
     DBUG_RETURN(-1);
@@ -784,9 +788,11 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp)
       DBUG_RETURN(-1);
     }
   }
-  // The rest of the frame are local variables which are all IN.
-  // Default all variables to null (those with default clauses will
-  // be set by an set instruction).
+  /*
+    The rest of the frame are local variables which are all IN.
+    Default all variables to null (those with default clauses will
+    be set by an set instruction).
+  */
   {
     Item_null *nit= NULL;	// Re-use this, and only create if needed
     for (; i < csize ; i++)
@@ -803,9 +809,11 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp)
 
   ret= execute(thd);
 
-  // Partially restore context now.
-  // We still need the call mem root and free list for processing
-  // of the result.
+  /*
+    Partially restore context now.
+    We still need the call mem root and free list for processing
+    of the result.
+  */
   thd->restore_backup_item_arena(&call_arena, &backup_arena);
 
   if (m_type == TYPE_ENUM_FUNCTION && ret == 0)
@@ -932,9 +940,11 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
       close_thread_tables(thd, 0, 0, 0);
 
     DBUG_PRINT("info",(" %.*s: eval args done", m_name.length, m_name.str));
-    // The rest of the frame are local variables which are all IN.
-    // Default all variables to null (those with default clauses will
-    // be set by an set instruction).
+    /*
+      The rest of the frame are local variables which are all IN.
+      Default all variables to null (those with default clauses will
+      be set by an set instruction).
+    */
     for (; i < csize ; i++)
     {
       if (! nit)
@@ -956,8 +966,10 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
     List_iterator<Item> li(*args);
     Item *it;
 
-    // Copy back all OUT or INOUT values to the previous frame, or
-    // set global user variables
+    /*
+      Copy back all OUT or INOUT values to the previous frame, or
+      set global user variables
+    */
     for (uint i = 0 ; (it= li++) && i < params ; i++)
     {
       sp_pvar_t *pvar= m_pcont->find_pvar(i);
@@ -987,8 +999,10 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
 	    octx->set_item(offset, copy);
 	  if (orig && copy == orig)
 	  {
-	    // A reused item slot, where the constructor put it in the
-	    // free_list, so we have to restore the list.
+	    /*
+              A reused item slot, where the constructor put it in the
+              free_list, so we have to restore the list.
+            */
 	    thd->free_list= o_free_list;
 	    copy->next= o_item_next;
 	  }
@@ -1420,8 +1434,6 @@ sp_head::opt_mark(uint ip)
     ip= i->opt_mark(this);
 }
 
-// ------------------------------------------------------------------
-
 
 /*
   Prepare LEX and thread for execution of instruction, if requested open
@@ -1513,6 +1525,7 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp,
 
   thd->proc_info="closing tables";
   close_thread_tables(thd);
+  thd->proc_info= 0;
 
   if (m_lex->query_tables_own_last)
   {
@@ -1549,9 +1562,10 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp,
 }
 
 
-//
-// sp_instr
-//
+/*
+  sp_instr class functions
+*/
+
 int sp_instr::exec_core(THD *thd, uint *nextp)
 {
   DBUG_ASSERT(0);
@@ -1559,9 +1573,10 @@ int sp_instr::exec_core(THD *thd, uint *nextp)
 }
 
 
-//
-// sp_instr_stmt
-//
+/*
+  sp_instr_stmt class functions
+*/
+
 int
 sp_instr_stmt::execute(THD *thd, uint *nextp)
 {
@@ -1606,9 +1621,11 @@ sp_instr_stmt::exec_core(THD *thd, uint *nextp)
   return res;
 }
 
-//
-// sp_instr_set
-//
+
+/*
+  sp_instr_set class functions
+*/
+
 int
 sp_instr_set::execute(THD *thd, uint *nextp)
 {
@@ -1618,6 +1635,7 @@ sp_instr_set::execute(THD *thd, uint *nextp)
   DBUG_RETURN(m_lex_keeper.reset_lex_and_exec_core(thd, nextp, TRUE, this));
 }
 
+
 int
 sp_instr_set::exec_core(THD *thd, uint *nextp)
 {
@@ -1638,9 +1656,10 @@ sp_instr_set::print(String *str)
 }
 
 
-//
-// sp_instr_set_trigger_field
-//
+/*
+  sp_instr_set_trigger_field class functions
+*/
+
 int
 sp_instr_set_trigger_field::execute(THD *thd, uint *nextp)
 {
@@ -1671,9 +1690,11 @@ sp_instr_set_trigger_field::print(String *str)
   value->print(str);
 }
 
-//
-// sp_instr_jump
-//
+
+/*
+ sp_instr_jump class functions
+*/
+
 int
 sp_instr_jump::execute(THD *thd, uint *nextp)
 {
@@ -1732,9 +1753,10 @@ sp_instr_jump::opt_move(uint dst, List<sp_instr> *bp)
   m_ip= dst;
 }
 
-//
-// sp_instr_jump_if
-//
+
+/*
+  sp_instr_jump_if class functions
+*/
 
 int
 sp_instr_jump_if::execute(THD *thd, uint *nextp)
@@ -1790,9 +1812,11 @@ sp_instr_jump_if::opt_mark(sp_head *sp)
   return m_ip+1;
 }
 
-//
-// sp_instr_jump_if_not
-//
+
+/*
+  sp_instr_jump_if_not class functions
+*/
+
 int
 sp_instr_jump_if_not::execute(THD *thd, uint *nextp)
 {
@@ -1823,6 +1847,7 @@ sp_instr_jump_if_not::exec_core(THD *thd, uint *nextp)
   return res;
 }
 
+
 void
 sp_instr_jump_if_not::print(String *str)
 {
@@ -1833,6 +1858,7 @@ sp_instr_jump_if_not::print(String *str)
   m_expr->print(str);
 }
 
+
 uint
 sp_instr_jump_if_not::opt_mark(sp_head *sp)
 {
@@ -1848,9 +1874,10 @@ sp_instr_jump_if_not::opt_mark(sp_head *sp)
   return m_ip+1;
 }
 
-//
-// sp_instr_freturn
-//
+
+/*
+  sp_instr_freturn class functions
+*/
 
 int
 sp_instr_freturn::execute(THD *thd, uint *nextp)
@@ -1889,9 +1916,10 @@ sp_instr_freturn::print(String *str)
   m_value->print(str);
 }
 
-//
-// sp_instr_hpush_jump
-//
+/*
+  sp_instr_hpush_jump class functions
+*/
+
 int
 sp_instr_hpush_jump::execute(THD *thd, uint *nextp)
 {
@@ -1935,9 +1963,11 @@ sp_instr_hpush_jump::opt_mark(sp_head *sp)
   return m_ip+1;
 }
 
-//
-// sp_instr_hpop
-//
+
+/*
+  sp_instr_hpop class functions
+*/
+
 int
 sp_instr_hpop::execute(THD *thd, uint *nextp)
 {
@@ -1962,9 +1992,10 @@ sp_instr_hpop::backpatch(uint dest, sp_pcontext *dst_ctx)
 }
 
 
-//
-// sp_instr_hreturn
-//
+/*
+  sp_instr_hreturn class functions
+*/
+
 int
 sp_instr_hreturn::execute(THD *thd, uint *nextp)
 {
@@ -1980,6 +2011,7 @@ sp_instr_hreturn::execute(THD *thd, uint *nextp)
   DBUG_RETURN(0);
 }
 
+
 void
 sp_instr_hreturn::print(String *str)
 {
@@ -1990,6 +2022,7 @@ sp_instr_hreturn::print(String *str)
     str->qs_append(m_dest);
 }
 
+
 uint
 sp_instr_hreturn::opt_mark(sp_head *sp)
 {
@@ -2003,9 +2036,10 @@ sp_instr_hreturn::opt_mark(sp_head *sp)
 }
 
 
-//
-// sp_instr_cpush
-//
+/*
+  sp_instr_cpush class functions
+*/
+
 int
 sp_instr_cpush::execute(THD *thd, uint *nextp)
 {
@@ -2015,15 +2049,18 @@ sp_instr_cpush::execute(THD *thd, uint *nextp)
   DBUG_RETURN(0);
 }
 
+
 void
 sp_instr_cpush::print(String *str)
 {
   str->append("cpush");
 }
 
-//
-// sp_instr_cpop
-//
+
+/*
+  sp_instr_cpop class functions
+*/
+
 int
 sp_instr_cpop::execute(THD *thd, uint *nextp)
 {
@@ -2033,6 +2070,7 @@ sp_instr_cpop::execute(THD *thd, uint *nextp)
   DBUG_RETURN(0);
 }
 
+
 void
 sp_instr_cpop::print(String *str)
 {
@@ -2047,9 +2085,11 @@ sp_instr_cpop::backpatch(uint dest, sp_pcontext *dst_ctx)
   m_count= m_ctx->diff_cursors(dst_ctx);
 }
 
-//
-// sp_instr_copen
-//
+
+/*
+  sp_instr_copen class functions
+*/
+
 int
 sp_instr_copen::execute(THD *thd, uint *nextp)
 {
@@ -2117,9 +2157,11 @@ sp_instr_copen::print(String *str)
   str->qs_append(m_cursor);
 }
 
-//
-// sp_instr_cclose
-//
+
+/*
+  sp_instr_cclose class functions
+*/
+
 int
 sp_instr_cclose::execute(THD *thd, uint *nextp)
 {
@@ -2135,6 +2177,7 @@ sp_instr_cclose::execute(THD *thd, uint *nextp)
   DBUG_RETURN(res);
 }
 
+
 void
 sp_instr_cclose::print(String *str)
 {
@@ -2143,9 +2186,11 @@ sp_instr_cclose::print(String *str)
   str->qs_append(m_cursor);
 }
 
-//
-// sp_instr_cfetch
-//
+
+/*
+  sp_instr_cfetch class functions
+*/
+
 int
 sp_instr_cfetch::execute(THD *thd, uint *nextp)
 {
@@ -2161,6 +2206,7 @@ sp_instr_cfetch::execute(THD *thd, uint *nextp)
   DBUG_RETURN(res);
 }
 
+
 void
 sp_instr_cfetch::print(String *str)
 {
@@ -2178,9 +2224,11 @@ sp_instr_cfetch::print(String *str)
   }
 }
 
-//
-// sp_instr_error
-//
+
+/*
+  sp_instr_error class functions
+*/
+
 int
 sp_instr_error::execute(THD *thd, uint *nextp)
 {
@@ -2191,6 +2239,7 @@ sp_instr_error::execute(THD *thd, uint *nextp)
   DBUG_RETURN(-1);
 }
 
+
 void
 sp_instr_error::print(String *str)
 {
@@ -2199,12 +2248,12 @@ sp_instr_error::print(String *str)
   str->qs_append(m_errcode);
 }
 
-/* ------------------------------------------------------------------ */
 
+/* ------------------------------------------------------------------ */
 
-//
-// Security context swapping
-//
+/*
+  Security context swapping
+*/
 
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
 void
@@ -2453,11 +2502,12 @@ sp_head::add_used_tables_to_table_list(THD *thd,
   DBUG_RETURN(result);
 }
 
+
 /*
- * Simple function for adding an explicetly named (systems) table to
- * the global table list, e.g. "mysql", "proc".
- *
- */
+  Simple function for adding an explicetly named (systems) table to
+  the global table list, e.g. "mysql", "proc".
+*/
+
 TABLE_LIST *
 sp_add_to_query_tables(THD *thd, LEX *lex,
 		       const char *db, const char *name,
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 5ff5e580f81..692eb2fccbe 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -1919,8 +1919,7 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter)
           has added its base tables after itself, adjust the boundary pointer
           accordingly.
         */
-        if (query_tables_last_own &&
-            query_tables_last_own == &(tables->next_global) &&
+        if (query_tables_last_own == &(tables->next_global) &&
             tables->view->query_tables)
           query_tables_last_own= tables->view->query_tables_last;
         
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index c1d4fae5253..51a2eadc132 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -1863,17 +1863,18 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
     }
 #endif
   case COM_REFRESH:
-    {
-      statistic_increment(thd->status_var.com_stat[SQLCOM_FLUSH],
-			  &LOCK_status);
-      ulong options= (ulong) (uchar) packet[0];
-      if (check_global_access(thd,RELOAD_ACL))
-	break;
-      mysql_log.write(thd,command,NullS);
-      if (!reload_acl_and_cache(thd, options, (TABLE_LIST*) 0, NULL))
-        send_ok(thd);
+  {
+    bool not_used;
+    statistic_increment(thd->status_var.com_stat[SQLCOM_FLUSH],
+                        &LOCK_status);
+    ulong options= (ulong) (uchar) packet[0];
+    if (check_global_access(thd,RELOAD_ACL))
       break;
-    }
+    mysql_log.write(thd,command,NullS);
+    if (!reload_acl_and_cache(thd, options, (TABLE_LIST*) 0, &not_used))
+      send_ok(thd);
+    break;
+  }
 #ifndef EMBEDDED_LIBRARY
   case COM_SHUTDOWN:
   {
@@ -3822,13 +3823,13 @@ end_with_restore_list:
     lex->no_write_to_binlog= 1;
   case SQLCOM_FLUSH:
   {
+    bool write_to_binlog;
     if (check_global_access(thd,RELOAD_ACL) || check_db_used(thd, all_tables))
       goto error;
     /*
       reload_acl_and_cache() will tell us if we are allowed to write to the
       binlog or not.
     */
-    bool write_to_binlog;
     if (!reload_acl_and_cache(thd, lex->type, first_table, &write_to_binlog))
     {
       /*
@@ -6383,13 +6384,13 @@ void add_join_natural(TABLE_LIST *a,TABLE_LIST *b)
     tables              Tables to flush (if any)
     write_to_binlog     Depending on 'options', it may be very bad to write the
                         query to the binlog (e.g. FLUSH SLAVE); this is a
-                        pointer where, if it is not NULL, reload_acl_and_cache()
-                        will put 0 if it thinks we really should not write to
-                        the binlog. Otherwise it will put 1.
+                        pointer where reload_acl_and_cache() will put 0 if
+                        it thinks we really should not write to the binlog.
+                        Otherwise it will put 1.
 
   RETURN
     0	 ok
-    !=0  error
+    !=0  error.  thd->killed or thd->net.report_error is set
 */
 
 bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables,
@@ -6475,10 +6476,10 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables,
       */
       tmp_write_to_binlog= 0;
       if (lock_global_read_lock(thd))
-	return 1;
+	return 1;                               // Killed
       result=close_cached_tables(thd,(options & REFRESH_FAST) ? 0 : 1,
                                  tables);
-      if (make_global_read_lock_block_commit(thd))
+      if (make_global_read_lock_block_commit(thd)) // Killed
       {
         /* Don't leave things in a half-locked state */
         unlock_global_read_lock(thd);
@@ -6500,7 +6501,10 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables,
   {
     tmp_write_to_binlog= 0;
     if (reset_master(thd))
+    {
       result=1;
+      thd->fatal_error();                       // Ensure client get error
+    }
   }
 #endif
 #ifdef OPENSSL
@@ -6522,8 +6526,7 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables,
 #endif
  if (options & REFRESH_USER_RESOURCES)
    reset_mqh((LEX_USER *) NULL);
- if (write_to_binlog)
-   *write_to_binlog= tmp_write_to_binlog;
+ *write_to_binlog= tmp_write_to_binlog;
  return result;
 }