Commit 12f91b1d authored by Davi Arnaut's avatar Davi Arnaut

Bug#44672: Assertion failed: thd->transaction.xid_state.xid.is_null()

The problem is that when a optimization of read-only transactions
(bypass 2-phase commit) was implemented, it removed the code that
reseted the XID once a transaction wasn't active anymore:

sql/sql_parse.cc:

-  bzero(&thd->transaction.stmt, sizeof(thd->transaction.stmt));
-  if (!thd->active_transaction())
-    thd->transaction.xid_state.xid.null();
+  thd->transaction.stmt.reset();

This mostly worked fine as the transaction commit and rollback
functions (in handler.cc) reset the XID once the transaction is
ended. But those functions wouldn't reset the XID in case of
a empty transaction, leading to a assertion when a new starting
a new XA transaction.

The solution is to ensure that the XID state is reset when empty
transactions are ended (by either commit or rollback). This is
achieved by reorganizing the code so that the transaction cleanup
routine is invoked whenever a transaction is ended.

mysql-test/r/xa.result:
  Add test case result for Bug#44672
mysql-test/t/xa.test:
  Add test case for Bug#44672
sql/handler.cc:
  Invoke transaction cleanup function whenever a transaction is
  ended. Move XID state reset logic to the transaction cleanup
  function.
sql/sql_class.h:
  Add XID state reset logic.
parent 46a4685c
......@@ -75,3 +75,9 @@ xa rollback 'a','c';
xa start 'a','c';
drop table t1;
End of 5.0 tests
xa start 'a';
xa end 'a';
xa rollback 'a';
xa start 'a';
xa end 'a';
xa rollback 'a';
......@@ -124,6 +124,17 @@ drop table t1;
--echo End of 5.0 tests
#
# Bug#44672: Assertion failed: thd->transaction.xid_state.xid.is_null()
#
xa start 'a';
xa end 'a';
xa rollback 'a';
xa start 'a';
xa end 'a';
xa rollback 'a';
# Wait till all disconnects are completed
--source include/wait_until_count_sessions.inc
......@@ -1073,6 +1073,13 @@ int ha_commit_trans(THD *thd, bool all)
user, or an implicit commit issued by a DDL.
*/
THD_TRANS *trans= all ? &thd->transaction.all : &thd->transaction.stmt;
/*
"real" is a nick name for a transaction for which a commit will
make persistent changes. E.g. a 'stmt' transaction inside a 'all'
transation is not 'real': even though it's possible to commit it,
the changes are not durable as they might be rolled back if the
enclosing 'all' transaction is rolled back.
*/
bool is_real_trans= all || thd->transaction.all.ha_list == 0;
Ha_trx_info *ha_info= trans->ha_list;
my_xid xid= thd->transaction.xid_state.xid.get_my_xid();
......@@ -1184,16 +1191,9 @@ end:
if (rw_trans)
start_waiting_global_read_lock(thd);
}
else if (all)
{
/*
A COMMIT of an empty transaction. There may be savepoints.
Destroy them. If the transaction is not empty
savepoints are cleared in ha_commit_one_phase()
or ha_rollback_trans().
*/
/* Free resources and perform other cleanup even for 'empty' transactions. */
else if (is_real_trans)
thd->transaction.cleanup();
}
#endif /* USING_TRANSACTIONS */
DBUG_RETURN(error);
}
......@@ -1206,6 +1206,13 @@ int ha_commit_one_phase(THD *thd, bool all)
{
int error=0;
THD_TRANS *trans=all ? &thd->transaction.all : &thd->transaction.stmt;
/*
"real" is a nick name for a transaction for which a commit will
make persistent changes. E.g. a 'stmt' transaction inside a 'all'
transation is not 'real': even though it's possible to commit it,
the changes are not durable as they might be rolled back if the
enclosing 'all' transaction is rolled back.
*/
bool is_real_trans=all || thd->transaction.all.ha_list == 0;
Ha_trx_info *ha_info= trans->ha_list, *ha_info_next;
DBUG_ENTER("ha_commit_one_phase");
......@@ -1227,8 +1234,6 @@ int ha_commit_one_phase(THD *thd, bool all)
}
trans->ha_list= 0;
trans->no_2pc=0;
if (is_real_trans)
thd->transaction.xid_state.xid.null();
if (all)
{
#ifdef HAVE_QUERY_CACHE
......@@ -1236,8 +1241,9 @@ int ha_commit_one_phase(THD *thd, bool all)
query_cache.invalidate(thd->transaction.changed_tables);
#endif
thd->variables.tx_isolation=thd->session_tx_isolation;
thd->transaction.cleanup();
}
if (is_real_trans)
thd->transaction.cleanup();
}
#endif /* USING_TRANSACTIONS */
DBUG_RETURN(error);
......@@ -1249,6 +1255,13 @@ int ha_rollback_trans(THD *thd, bool all)
int error=0;
THD_TRANS *trans=all ? &thd->transaction.all : &thd->transaction.stmt;
Ha_trx_info *ha_info= trans->ha_list, *ha_info_next;
/*
"real" is a nick name for a transaction for which a commit will
make persistent changes. E.g. a 'stmt' transaction inside a 'all'
transation is not 'real': even though it's possible to commit it,
the changes are not durable as they might be rolled back if the
enclosing 'all' transaction is rolled back.
*/
bool is_real_trans=all || thd->transaction.all.ha_list == 0;
DBUG_ENTER("ha_rollback_trans");
......@@ -1294,18 +1307,13 @@ int ha_rollback_trans(THD *thd, bool all)
}
trans->ha_list= 0;
trans->no_2pc=0;
if (is_real_trans)
{
if (thd->transaction_rollback_request)
if (is_real_trans && thd->transaction_rollback_request)
thd->transaction.xid_state.rm_error= thd->main_da.sql_errno();
else
thd->transaction.xid_state.xid.null();
}
if (all)
thd->variables.tx_isolation=thd->session_tx_isolation;
}
/* Always cleanup. Even if there nht==0. There may be savepoints. */
if (all)
if (is_real_trans)
thd->transaction.cleanup();
#endif /* USING_TRANSACTIONS */
if (all)
......
......@@ -1464,6 +1464,14 @@ public:
{
changed_tables= 0;
savepoints= 0;
/*
If rm_error is raised, it means that this piece of a distributed
transaction has failed and must be rolled back. But the user must
rollback it explicitly, so don't start a new distributed XA until
then.
*/
if (!xid_state.rm_error)
xid_state.xid.null();
#ifdef USING_TRANSACTIONS
free_root(&mem_root,MYF(MY_KEEP_PREALLOC));
#endif
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment