Bug#24989: The DEADLOCK error is improperly handled by InnoDB.

When innodb detects a deadlock it calls ha_rollback_trans() to rollback the 
main transaction. But such action isn't allowed from inside of triggers and
functions. When it happen the 'Explicit or implicit commit' error is thrown
even if there is no commit/rollback statements in the trigger/function. This
leads to the user confusion.

Now the convert_error_code_to_mysql() function doesn't call the 
ha_rollback_trans() function directly but rather calls the
mark_transaction_to_rollback function and returns an error.
The sp_rcontext::find_handler() now doesn't allow errors to be caught by the
trigger/function error handlers when the thd->is_fatal_sub_stmt_error flag
is set. Procedures are still allowed to catch such errors.
The sp_rcontext::find_handler function now accepts a THD handle as a parameter.
The transaction_rollback_request and the is_fatal_sub_stmt_error flags are 
added to the THD class. The are initialized by the THD class constructor.
Now the ha_autocommit_or_rollback function rolls back main transaction
when not in a sub statement and the thd->transaction_rollback_request
is set.
The THD::restore_sub_statement_state function now resets the 
thd->is_fatal_sub_stmt_error flag on exit from a sub-statement.


sql/ha_innodb.cc:
  Bug#24989: The DEADLOCK error is improperly handled by InnoDB.
  Now the convert_error_code_to_mysql() function doesn't call the
  ha_rollback_trans() function directly but rather calls the
  mark_transaction_to_rollback function and returns an error.
sql/handler.cc:
  Bug#24989: The DEADLOCK error is improperly handled by InnoDB.
  Now the ha_autocommit_or_rollback function rolls back main transaction
  when not in a sub statement and the thd->transaction_rollback_request
  is set.
mysql-test/r/innodb-big.result:
  Added a test case for the bug#24989: The DEADLOCK error is improperly handled by
  InnoDB.
mysql-test/t/innodb-big.test:
  Added a test case for the bug#24989: The DEADLOCK error is improperly handled by
  InnoDB.
sql/sql_class.h:
  Bug#24989: The DEADLOCK error is improperly handled by InnoDB.
  The transaction_rollback_request and the is_fatal_sub_stmt_error flags are 
  added to the THD class.
sql/sql_class.cc:
  Bug#24989: The DEADLOCK error is improperly handled by InnoDB.
  Initialization of the transaction_rollback_request and the
  is_fatal_sub_stmt_error flags are added to the THD class constructor.
  The mark_transaction_to_rollback function is added.
  The THD::restore_sub_statement_state function now resets the
  thd->is_fatal_sub_stmt_error flag on exit from a sub-statement.
sql/sp_rcontext.h:
  Bug#24989: The DEADLOCK error is improperly handled by InnoDB.
  The sp_rcontext::find_handler function now accepts a THD handle as a parameter.
  The in_sub_stmt flag is added to the sp_rcontext class.
sql/sp_rcontext.cc:
  Bug#24989: The DEADLOCK error is improperly handled by InnoDB.
  The sp_rcontext::find_handler() now doesn't allow errors to be caught by the
  trigger/function error handlers when the thd->is_fatal_sub_stmt_error flag
  is set. Instead it tries to find a most inner procedure that isn't called
  directly or indirectly from any function/trigger.
  Procedures are still allowed to catch such errors.
  The sp_rcontext::find_handler function now accepts a THD handle as a parameter.

6 files changed, 78 insertions, 15 deletions

diff --git a/sql/ha_innodb.cc b/sql/ha_innodb.cc
index 455c0968050..d4a43dc4297 100644
--- a/sql/ha_innodb.cc
+++ b/sql/ha_innodb.cc
@@ -455,9 +455,7 @@ convert_error_code_to_mysql(
  		tell it also to MySQL so that MySQL knows to empty the
  		cached binlog for this transaction */
 
- 		if (thd) {
- 			ha_rollback(thd);
- 		}
+                mark_transaction_to_rollback(thd, TRUE);
 
     		return(HA_ERR_LOCK_DEADLOCK);
 
@@ -467,9 +465,7 @@ convert_error_code_to_mysql(
 		latest SQL statement in a lock wait timeout. Previously, we
 		rolled back the whole transaction. */
 
-		if (thd && row_rollback_on_timeout) {
-			ha_rollback(thd);
-		}
+                mark_transaction_to_rollback(thd, row_rollback_on_timeout);
 
    		return(HA_ERR_LOCK_WAIT_TIMEOUT);
 
@@ -521,9 +517,7 @@ convert_error_code_to_mysql(
  		tell it also to MySQL so that MySQL knows to empty the
  		cached binlog for this transaction */
 
- 		if (thd) {
- 			ha_rollback(thd);
- 		}
+                mark_transaction_to_rollback(thd, TRUE);
 
     		return(HA_ERR_LOCK_TABLE_FULL);
     	} else {
diff --git a/sql/handler.cc b/sql/handler.cc
index 867ac7ff778..e4a5e65b80c 100644
--- a/sql/handler.cc
+++ b/sql/handler.cc
@@ -821,6 +821,9 @@ int ha_rollback_trans(THD *thd, bool all)
     }
   }
 #endif /* USING_TRANSACTIONS */
+  if (all)
+    thd->transaction_rollback_request= FALSE;
+
   /*
     If a non-transactional table was updated, warn; don't warn if this is a
     slave thread (because when a slave thread executes a ROLLBACK, it has
@@ -858,6 +861,8 @@ int ha_autocommit_or_rollback(THD *thd, int error)
       if (ha_commit_stmt(thd))
 	error=1;
     }
+    else if (thd->transaction_rollback_request && !thd->in_sub_stmt)
+      (void) ha_rollback(thd);
     else
       (void) ha_rollback_stmt(thd);
 
diff --git a/sql/sp_rcontext.cc b/sql/sp_rcontext.cc
index e49c4eb1240..ac7c46c9fe5 100644
--- a/sql/sp_rcontext.cc
+++ b/sql/sp_rcontext.cc
@@ -37,6 +37,7 @@ sp_rcontext::sp_rcontext(sp_pcontext *root_parsing_ctx,
    m_var_items(0),
    m_return_value_fld(return_value_fld),
    m_return_value_set(FALSE),
+   in_sub_stmt(FALSE),
    m_hcount(0),
    m_hsp(0),
    m_ihsp(0),
@@ -67,6 +68,8 @@ sp_rcontext::~sp_rcontext()
 
 bool sp_rcontext::init(THD *thd)
 {
+  in_sub_stmt= thd->in_sub_stmt;
+
   if (init_var_table(thd) || init_var_items())
     return TRUE;
 
@@ -191,7 +194,7 @@ sp_rcontext::set_return_value(THD *thd, Item **return_value_item)
 */
 
 bool
-sp_rcontext::find_handler(uint sql_errno,
+sp_rcontext::find_handler(THD *thd, uint sql_errno,
                           MYSQL_ERROR::enum_warning_level level)
 {
   if (m_hfound >= 0)
@@ -200,6 +203,15 @@ sp_rcontext::find_handler(uint sql_errno,
   const char *sqlstate= mysql_errno_to_sqlstate(sql_errno);
   int i= m_hcount, found= -1;
 
+  /*
+    If this is a fatal sub-statement error, and this runtime
+    context corresponds to a sub-statement, no CONTINUE/EXIT
+    handlers from this context are applicable: try to locate one
+    in the outer scope.
+  */
+  if (thd->is_fatal_sub_stmt_error && in_sub_stmt)
+    i= 0;
+
   /* Search handlers from the latest (innermost) to the oldest (outermost) */
   while (i--)
   {
@@ -252,7 +264,7 @@ sp_rcontext::find_handler(uint sql_errno,
     */
     if (m_prev_runtime_ctx && IS_EXCEPTION_CONDITION(sqlstate) &&
         level == MYSQL_ERROR::WARN_LEVEL_ERROR)
-      return m_prev_runtime_ctx->find_handler(sql_errno, level);
+      return m_prev_runtime_ctx->find_handler(thd, sql_errno, level);
     return FALSE;
   }
   m_hfound= found;
@@ -298,7 +310,7 @@ sp_rcontext::handle_error(uint sql_errno,
     elevated_level= MYSQL_ERROR::WARN_LEVEL_ERROR;
   }
 
-  if (find_handler(sql_errno, elevated_level))
+  if (find_handler(thd, sql_errno, elevated_level))
   {
     if (elevated_level == MYSQL_ERROR::WARN_LEVEL_ERROR)
     {
diff --git a/sql/sp_rcontext.h b/sql/sp_rcontext.h
index fbf479f52aa..0104b71a648 100644
--- a/sql/sp_rcontext.h
+++ b/sql/sp_rcontext.h
@@ -125,7 +125,7 @@ class sp_rcontext : public Sql_alloc
 
   // Returns 1 if a handler was found, 0 otherwise.
   bool
-  find_handler(uint sql_errno,MYSQL_ERROR::enum_warning_level level);
+  find_handler(THD *thd, uint sql_errno,MYSQL_ERROR::enum_warning_level level);
 
   // If there is an error handler for this error, handle it and return TRUE.
   bool
@@ -236,6 +236,10 @@ private:
     during execution.
   */
   bool m_return_value_set;
+  /**
+    TRUE if the context is created for a sub-statement.
+  */
+  bool in_sub_stmt;
 
   sp_handler_t *m_handler;      // Visible handlers
   uint m_hcount;                // Stack pointer for m_handler
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 010dc101e0d..33286e814aa 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -173,6 +173,7 @@ THD::THD()
    Open_tables_state(refresh_version),
    lock_id(&main_lock_id),
    user_time(0), in_sub_stmt(0), global_read_lock(0), is_fatal_error(0),
+   transaction_rollback_request(0), is_fatal_sub_stmt_error(0),
    rand_used(0), time_zone_used(0),
    last_insert_id_used(0), last_insert_id_used_bin_log(0), insert_id_used(0),
    clear_next_insert_id(0), in_lock_tables(0), bootstrap(0),
@@ -967,7 +968,7 @@ void select_send::abort()
 {
   DBUG_ENTER("select_send::abort");
   if (status && thd->spcont &&
-      thd->spcont->find_handler(thd->net.last_errno,
+      thd->spcont->find_handler(thd, thd->net.last_errno,
                                 MYSQL_ERROR::WARN_LEVEL_ERROR))
   {
     /*
@@ -2150,6 +2151,13 @@ void THD::restore_sub_statement_state(Sub_statement_state *backup)
   limit_found_rows= backup->limit_found_rows;
   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.
+  */
+  if (!in_sub_stmt)
+    is_fatal_sub_stmt_error= FALSE;
 
   if ((options & OPTION_BIN_LOG) && is_update_query(lex->sql_command))
     mysql_bin_log.stop_union_events(this);
@@ -2163,6 +2171,18 @@ void THD::restore_sub_statement_state(Sub_statement_state *backup)
 }
 
 
+/**
+  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)
+{
+  thd->is_fatal_sub_stmt_error= TRUE;
+  thd->transaction_rollback_request= all;
+}
 /***************************************************************************
   Handling of XA id cacheing
 ***************************************************************************/
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 5f813e82307..674409d6f08 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -1406,7 +1406,33 @@ public:
   bool       slave_thread, one_shot_set;
   bool	     locked, some_tables_deleted;
   bool       last_cuted_field;
-  bool	     no_errors, password, is_fatal_error;
+  bool	     no_errors, password;
+  /**
+    Set to TRUE if execution of the current compound statement
+    can not continue. In particular, disables activation of
+    CONTINUE or EXIT handlers of stored routines.
+    Reset in the end of processing of the current user request, in
+    @see mysql_reset_thd_for_next_command().
+  */
+  bool is_fatal_error;
+  /**
+    Set by a storage engine to request the entire
+    transaction (that possibly spans multiple engines) to
+    rollback. Reset in ha_rollback.
+  */
+  bool       transaction_rollback_request;
+  /**
+    TRUE if we are in a sub-statement and the current error can
+    not be safely recovered until we left the sub-statement mode.
+    In particular, disables activation of CONTINUE and EXIT
+    handlers inside sub-statements. E.g. if it is a deadlock
+    error and requires a transaction-wide rollback, this flag is
+    raised (traditionally, MySQL first has to close all the reads
+    via @see handler::ha_index_or_rnd_end() and only then perform
+    the rollback).
+    Reset to FALSE when we leave the sub-statement mode.
+  */
+  bool       is_fatal_sub_stmt_error;
   bool	     query_start_used, rand_used, time_zone_used;
 
   /*
@@ -2338,3 +2364,5 @@ public:
 /* Functions in sql_class.cc */
 
 void add_to_status(STATUS_VAR *to_var, STATUS_VAR *from_var);
+void mark_transaction_to_rollback(THD *thd, bool all);
+