Bug#16041903 CONTINUE HANDLER NOT INVOKED IN A STORED

FUNCTION AFTER A LOCK WAIT TIMEOUT

Overview
--------
When the SQL layer is executing a sub statement (stored
function or trigger), some errors cannot be safely recovered
until we leave sub statement mode. This is the case for
e.g. ER_LOCK_WAIT_TIMEOUT. When this error is caught, the
handler does a callback to "mark_transaction_for_rollback()"
in the SQL layer, which makes the transaction be rolled back
implicitly. In "mark_transaction_for_rollback()", the flag
"THD::is_fatal_sub_stmt_error" is set. This flag is used to
effectievly disable the execution of a condition handler in
a stored function while handling the error. The flag is
reset when leaving sub statement mode, in
"restore_sub_statement_state()", if leaving the last level
of sub statements (i.e., nested sub statements are handled
by keeping the value of the flag when going to the previous
"stack frame"). 

The problem here is that the flag is set in
"mark_transaction_for_rollback()" even if the SQL layer is
not in sub statement mode. If this happens, and the
transaction afterwards calls a stored function with a
condition handler, the handler will be disabled since the
flag is already set when calling the function.


Suggested fix
-------------
The suggested fix is to set "is_fatal_sub_stmt_error"
in "mark_transaction_for_rollback()" only in the case where
the SQL layer is currently executing a sub statement
("in_sub_stmt==true"). If not in a sub statement,
setting the "is_fatal_sub_stmt_error" flag really has
no meaning. Additionally, when the flag is set when the SQL
layer is not actually in a sub statement, the flag is never
cleared since this happens only in
"THD::restore_sub_statement_state()" when leaving sub
statement mode.

Author: ssorumgard

sql/sql_base.cc

@@ -3949,7 +3949,7 @@ request_backoff_action(enum_open_table_action action_arg,
   if (action_arg != OT_REOPEN_TABLES && m_has_locks)
   {
     my_error(ER_LOCK_DEADLOCK, MYF(0));
-    mark_transaction_to_rollback(m_thd, true);
+    m_thd->mark_transaction_to_rollback(true);
     return TRUE;
   }
   /*

sql/sql_class.cc

@@ -861,7 +861,7 @@ THD::THD(bool enable_plugins)
    next_to_commit(NULL),
    is_fatal_error(0),
    transaction_rollback_request(0),
-   is_fatal_sub_stmt_error(0),
+   is_fatal_sub_stmt_error(false),
    rand_used(0),
    time_zone_used(0),
    in_lock_tables(0),
@@ -3973,7 +3973,8 @@ extern "C" int thd_binlog_format(const MYSQL_THD thd)
 
 extern "C" void thd_mark_transaction_to_rollback(MYSQL_THD thd, bool all)
 {
-  mark_transaction_to_rollback(thd, all);
+  DBUG_ASSERT(thd);
+  thd->mark_transaction_to_rollback(all);
 }
 
 extern "C" bool thd_binlog_filter_ok(const MYSQL_THD thd)
@@ -4201,13 +4202,21 @@ void THD::restore_sub_statement_state(Sub_statement_state *backup)
   limit_found_rows= backup->limit_found_rows;
   set_sent_row_count(backup->sent_row_count);
   client_capabilities= backup->client_capabilities;
+
   /*
     If we've left sub-statement mode, reset the fatal error flag.
     Otherwise keep the current value, to propagate it up the sub-statement
     stack.
+
+    NOTE: is_fatal_sub_stmt_error can be set only if we've been in the
+    sub-statement mode.
   */
+
+  DBUG_ASSERT((is_fatal_sub_stmt_error && !in_sub_stmt) ||
+              !is_fatal_sub_stmt_error);
+
   if (!in_sub_stmt)
-    is_fatal_sub_stmt_error= FALSE;
+    is_fatal_sub_stmt_error= false;
 
   if ((variables.option_bits & OPTION_BIN_LOG) && is_update_query(lex->sql_command) &&
        !is_current_stmt_binlog_format_row())
@@ -4460,27 +4469,30 @@ void THD::get_definer(LEX_USER *definer)
 /**
   Mark transaction to rollback and mark error as fatal to a sub-statement.
 
-  @param  thd   Thread handle
   @param  all   TRUE <=> rollback main transaction.
 */
 
-void mark_transaction_to_rollback(THD *thd, bool all)
+void THD::mark_transaction_to_rollback(bool all)
 {
-  if (thd)
-  {
-    thd->is_fatal_sub_stmt_error= TRUE;
-    thd->transaction_rollback_request= all;
-    /*
-      Aborted transactions can not be IGNOREd.
-      Switch off the IGNORE flag for the current
-      SELECT_LEX. This should allow my_error()
-      to report the error and abort the execution
-      flow, even in presence
-      of IGNORE clause.
-    */
-    if (thd->lex->current_select())
-      thd->lex->current_select()->no_error= FALSE;
-  }
+  /*
+    There is no point in setting is_fatal_sub_stmt_error unless
+    we are actually in_sub_stmt.
+  */
+  if (in_sub_stmt)
+    is_fatal_sub_stmt_error= true;
+
+  transaction_rollback_request= all;
+
+  /*
+    Aborted transactions can not be IGNOREd.
+    Switch off the IGNORE flag for the current
+    SELECT_LEX. This should allow my_error()
+    to report the error and abort the execution
+    flow, even in presence
+    of IGNORE clause.
+  */
+  if (lex->current_select())
+    lex->current_select()->no_error= false;
 }
 
 

sql/sql_class.h

@@ -637,9 +637,6 @@ typedef struct system_status_var
 
 #define last_system_status_var questions
 
-void mark_transaction_to_rollback(THD *thd, bool all);
-
-
 /**
   Get collation by name, send error to client on failure.
   @param name     Collation name
@@ -3913,6 +3910,8 @@ class THD :public MDL_context_owner,
   LEX_STRING get_invoker_host() { return invoker_host; }
   bool has_invoker() { return invoker_user.length > 0; }
 
+  void mark_transaction_to_rollback(bool all);
+
 #ifndef DBUG_OFF
 private:
   int gis_debug; // Storage for "SELECT ST_GIS_DEBUG(param);"
@@ -5252,7 +5251,6 @@ void add_to_status(STATUS_VAR *to_var, STATUS_VAR *from_var);
 
 void add_diff_to_status(STATUS_VAR *to_var, STATUS_VAR *from_var,
                         STATUS_VAR *dec_var);
-void mark_transaction_to_rollback(THD *thd, bool all);
 
 /* Inline functions */