From 54769c281ec9cf7d52740ed5ca503c5d81ed1744 Mon Sep 17 00:00:00 2001 From: Dmitry Shulga Date: Thu, 6 Dec 2012 15:59:15 +0600 Subject: This patch fixes bug#14729757 - MY_HASH_SEARCH(&XID_CACHE, XID_STATE->XID.KEY(), XID_STATE->XID.KEY_LENGTH())==0 This bug is a regression of bug#11759534 - 51855: RACE CONDITION IN XA START. The reason for regression is that the changes that fixes the original bug wasn't merged from mysql-5.1 into mysql-5.5 and mysql-trunk. Only null-merge was done for the patch changeset. To incorporate lost changes the manual merge have been done. Additionally the call of trans_rolback() was added into trans_xa_start() in case if xid_cache_insert is failed() after transaction has been started. If we don't call trans_rollback() we would never reset the flag SERVER_STATUS_IN_TRANS in THD::server_status and therefore all subsequent attempts to execute XA START in the connection where the error was occurred will be failed since thd->in_active_multi_stmt_transaction() will return the true every time when trans_xa_start is called. The latest changes were absent in patch for mysql-5.1 --- sql/sql_class.cc | 11 ++++++++--- sql/transaction.cc | 20 +++++++++++++++++--- 2 files changed, 25 insertions(+), 6 deletions(-) (limited to 'sql') diff --git a/sql/sql_class.cc b/sql/sql_class.cc index ff7f2340ac0..6e47dcb5795 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -4024,9 +4024,14 @@ bool xid_cache_insert(XID *xid, enum xa_states xa_state) bool xid_cache_insert(XID_STATE *xid_state) { mysql_mutex_lock(&LOCK_xid_cache); - DBUG_ASSERT(my_hash_search(&xid_cache, xid_state->xid.key(), - xid_state->xid.key_length())==0); - my_bool res=my_hash_insert(&xid_cache, (uchar*)xid_state); + if (my_hash_search(&xid_cache, xid_state->xid.key(), + xid_state->xid.key_length())) + { + mysql_mutex_unlock(&LOCK_xid_cache); + my_error(ER_XAER_DUPID, MYF(0)); + return true; + } + bool res= my_hash_insert(&xid_cache, (uchar*)xid_state); mysql_mutex_unlock(&LOCK_xid_cache); return res; } diff --git a/sql/transaction.cc b/sql/transaction.cc index 3359decbcd5..1623cd57d77 100644 --- a/sql/transaction.cc +++ b/sql/transaction.cc @@ -567,15 +567,19 @@ bool trans_xa_start(THD *thd) my_error(ER_XAER_RMFAIL, MYF(0), xa_state_names[xa_state]); else if (thd->locked_tables_mode || thd->in_active_multi_stmt_transaction()) my_error(ER_XAER_OUTSIDE, MYF(0)); - else if (xid_cache_search(thd->lex->xid)) - my_error(ER_XAER_DUPID, MYF(0)); else if (!trans_begin(thd)) { DBUG_ASSERT(thd->transaction.xid_state.xid.is_null()); thd->transaction.xid_state.xa_state= XA_ACTIVE; thd->transaction.xid_state.rm_error= 0; thd->transaction.xid_state.xid.set(thd->lex->xid); - xid_cache_insert(&thd->transaction.xid_state); + if (xid_cache_insert(&thd->transaction.xid_state)) + { + thd->transaction.xid_state.xa_state= XA_NOTR; + thd->transaction.xid_state.xid.null(); + trans_rollback(thd); + DBUG_RETURN(true); + } DBUG_RETURN(FALSE); } @@ -661,6 +665,16 @@ bool trans_xa_commit(THD *thd) if (!thd->transaction.xid_state.xid.eq(thd->lex->xid)) { + /* + xid_state.in_thd is always true beside of xa recovery procedure. + Note, that there is no race condition here between xid_cache_search + and xid_cache_delete, since we always delete our own XID + (thd->lex->xid == thd->transaction.xid_state.xid). + The only case when thd->lex->xid != thd->transaction.xid_state.xid + and xid_state->in_thd == 0 is in the function + xa_cache_insert(XID, xa_states), which is called before starting + client connections, and thus is always single-threaded. + */ XID_STATE *xs= xid_cache_search(thd->lex->xid); res= !xs || xs->in_thd; if (res) -- cgit v1.2.1