List:Commits« Previous MessageNext Message »
From:Sergey Vojtovich Date:June 21 2010 11:50am
Subject:Re: bzr commit into mysql-5.1 branch (Dmitry.Shulga:3411) Bug#51855
View as plain text  
Dmitry,

as discussed on IRC todo is as following:
1. Check if it is doable and reasonable to fix this bug by refusing
duplicate elements in xid_cache (HASH_UNIQUE flag). If so, try to fix it
this way.

2. We should give proof of need to move members to XA_INFO class. If
there is a problem indeed, we should probably report another bug. Or
whatever, I'd prefer to see a test case before proposing any solution.

Regards,
Sergey

On Sat, Jun 19, 2010 at 04:50:54PM +0400, Sergey Vojtovich wrote:
> Hi Dmitry,
> 
> thanks for the patch. Though a few questions:
> 
> IMHO there is a cleaner solution: initialize hash with HASH_UNIQUE, this
> way hash_insert forbids duplicate keys. In other words works mostly the
> same way as xid_cache_find_or_insert(). Was this approach considered? If
> it was, why it was abandoned?
> 
> What are the reasons of move of XID_STATE members to a separate class?
> It is not obvious from cset comment.
> 
> Prior to this patch failure to add new element to the hash (hash_insert
> OOM) was simply ignored. With this patch it is considered as duplicate
> XID. I guess this is not intentional, right?
> 
> Taking into account that failure to add new element to the hash is
> handled now, shouldn't we in this case reset XID and other members
> like xa_state?
> 
> Thanks,
> Sergey
> 
> On Fri, Jun 18, 2010 at 09:26:36AM +0000, Dmitry Shulga wrote:
> > #At file:///Users/shulga/projects/mysql/5.1-bug51855/ based on
> revid:build@stripped
> > 
> >  3411 Dmitry Shulga	2010-06-18
> >       Fixed bug #51855. Race condition in XA START. If several threads
> concurrently executing statements block like this
> >       XA START 'x';
> >       XA END 'x';
> >       XA PREPARE 'x';
> >       XA COMMIT 'x';
> >       then mysqld server (built with debug) could crash.
> >       Fixed race condition in XA COMMIT/XA ROLLBACK.
> >      @ sql/handler.cc
> >         XID_STATE's attributes xa_state/in_thd/rm_error moved to separate class
> XA_INFO and reference to ones made as value.xa_state/value.in_thd/value.rm_error.
> >      @ sql/set_var.cc
> >         XID_STATE's attributes xa_state/in_thd/rm_error moved to separate class
> XA_INFO and reference to ones made as value.xa_state/value.in_thd/value.rm_error.
> >      @ sql/sql_class.cc
> >         added xid_cache_find_or_insert standalone procedure definition;
> xid_cache_search now returns XA_INFO value in second argument and returns bool value as
> search result (false if cache entry not found, else return true); changed argument typr
> for xid_cache_delete - now it have type XID_STATE*.
> >      @ sql/sql_class.h
> >         added declaration for xid_cache_find_or_insert; modified signature for
> xid_cache_delete and xid_cache_serach; modified definition for XID_STATE - attributes
> xa_state, in_thd, rm_error moved to separate struct XA_INFO and value of this struct
> included as field to XID_STATE.
> >      @ sql/sql_parse.cc
> >         sequence of calls to xid_cache_search(..) xid_cache_insert(...) replaced
> by call to xid_cache_find_or_insert(...); XID_STATE's attributes xa_state/in_thd/rm_error
> moved to separate class XA_INFO and reference to ones made as
> value.xa_state/value.in_thd/value.rm_error; at case SQLCOM_XA_COMMIT/SQLCOM_XA_ROLLBACK:
> get information about XA state by value when calling to xid_cache_search.
> > 
> >     modified:
> >       sql/handler.cc
> >       sql/set_var.cc
> >       sql/sql_class.cc
> >       sql/sql_class.h
> >       sql/sql_parse.cc
> > === modified file 'sql/handler.cc'
> > --- a/sql/handler.cc	2010-04-14 09:53:59 +0000
> > +++ b/sql/handler.cc	2010-06-18 09:26:33 +0000
> > @@ -1316,8 +1316,8 @@ int ha_rollback_trans(THD *thd, bool all
> >      trans->ha_list= 0;
> >      trans->no_2pc=0;
> >      if (is_real_trans && thd->transaction_rollback_request
> &&
> > -        thd->transaction.xid_state.xa_state != XA_NOTR)
> > -      thd->transaction.xid_state.rm_error= thd->main_da.sql_errno();
> > +        thd->transaction.xid_state.value.xa_state != XA_NOTR)
> > +      thd->transaction.xid_state.value.rm_error=
> thd->main_da.sql_errno();
> >      if (all)
> >        thd->variables.tx_isolation=thd->session_tx_isolation;
> >    }
> > @@ -1655,7 +1655,7 @@ bool mysql_xa_recover(THD *thd)
> >    pthread_mutex_lock(&LOCK_xid_cache);
> >    while ((xs= (XID_STATE*)hash_element(&xid_cache, i++)))
> >    {
> > -    if (xs->xa_state==XA_PREPARED)
> > +    if (xs->value.xa_state==XA_PREPARED)
> >      {
> >        protocol->prepare_for_resend();
> >        protocol->store_longlong((longlong)xs->xid.formatID, FALSE);
> > 
> > === modified file 'sql/set_var.cc'
> > --- a/sql/set_var.cc	2010-05-20 06:31:03 +0000
> > +++ b/sql/set_var.cc	2010-06-18 09:26:33 +0000
> > @@ -3082,11 +3082,11 @@ static bool set_option_autocommit(THD *t
> >      if ((org_options & OPTION_NOT_AUTOCOMMIT))
> >      {
> >        /* We changed to auto_commit mode */
> > -      if (thd->transaction.xid_state.xa_state != XA_NOTR)
> > +      if (thd->transaction.xid_state.value.xa_state != XA_NOTR)
> >        {
> >          thd->options= org_options;
> >          my_error(ER_XAER_RMFAIL, MYF(0),
> > -                 xa_state_names[thd->transaction.xid_state.xa_state]);
> > +                
> xa_state_names[thd->transaction.xid_state.value.xa_state]);
> >          return 1;
> >        }
> >        thd->options&= ~(ulonglong) (OPTION_BEGIN | OPTION_KEEP_LOG);
> > 
> > === modified file 'sql/sql_class.cc'
> > --- a/sql/sql_class.cc	2010-05-08 03:25:33 +0000
> > +++ b/sql/sql_class.cc	2010-06-18 09:26:33 +0000
> > @@ -898,7 +898,7 @@ void THD::init_for_queries()
> >                        variables.trans_prealloc_size);
> >  #endif
> >    transaction.xid_state.xid.null();
> > -  transaction.xid_state.in_thd=1;
> > +  transaction.xid_state.value.in_thd=1;
> >  }
> >  
> >  
> > @@ -948,7 +948,7 @@ void THD::cleanup(void)
> >  #endif
> >    {
> >      ha_rollback(this);
> > -    xid_cache_delete(&transaction.xid_state);
> > +    xid_cache_delete(&transaction.xid_state.xid);
> >    }
> >    if (locked_tables)
> >    {
> > @@ -3312,7 +3312,7 @@ uchar *xid_get_hash_key(const uchar *ptr
> >  
> >  void xid_free_hash(void *ptr)
> >  {
> > -  if (!((XID_STATE*)ptr)->in_thd)
> > +  if (!((XID_STATE*)ptr)->value.in_thd)
> >      my_free((uchar*)ptr, MYF(0));
> >  }
> >  
> > @@ -3332,15 +3332,41 @@ void xid_cache_free()
> >    }
> >  }
> >  
> > -XID_STATE *xid_cache_search(XID *xid)
> > +/*
> > +  return true if xid_state has been inserted to HASH, else return false
> > +*/
> > +bool xid_cache_find_or_insert(XID *xid, XID_STATE *xid_state,
> > +                              enum xa_states xa_state)
> >  {
> >    pthread_mutex_lock(&LOCK_xid_cache);
> > -  XID_STATE *res=(XID_STATE *)hash_search(&xid_cache, xid->key(),
> xid->key_length());
> > +  bool res = false;
> > +  if (!hash_search(&xid_cache, xid->key(), xid->key_length()))
> > +  {
> > +    DBUG_ASSERT(xid_state->xid.is_null());
> > +    xid_state->value.xa_state=xa_state;
> > +    xid_state->value.rm_error= 0;
> > +    xid_state->xid.set(xid);
> > +    res = (my_hash_insert(&xid_cache, (uchar*)xid_state) == 0);
> > +  }
> >    pthread_mutex_unlock(&LOCK_xid_cache);
> >    return res;
> >  }
> >  
> >  
> > +bool xid_cache_search(XID *xid, XA_INFO* value)
> > +{
> > +  pthread_mutex_lock(&LOCK_xid_cache);
> > +  XID_STATE *res=(XID_STATE *)hash_search(&xid_cache, xid->key(),
> xid->key_length());
> > +  if (res) {
> > +    *value = res->value;
> > +    pthread_mutex_unlock(&LOCK_xid_cache);
> > +    return true;
> > +  }
> > +  pthread_mutex_unlock(&LOCK_xid_cache);
> > +  return false;
> > +}
> > +
> > +
> >  bool xid_cache_insert(XID *xid, enum xa_states xa_state)
> >  {
> >    XID_STATE *xs;
> > @@ -3352,9 +3378,9 @@ bool xid_cache_insert(XID *xid, enum xa_
> >      res=1;
> >    else
> >    {
> > -    xs->xa_state=xa_state;
> > +    xs->value.xa_state=xa_state;
> >      xs->xid.set(xid);
> > -    xs->in_thd=0;
> > +    xs->value.in_thd=0;
> >      res=my_hash_insert(&xid_cache, (uchar*)xs);
> >    }
> >    pthread_mutex_unlock(&LOCK_xid_cache);
> > @@ -3373,10 +3399,12 @@ bool xid_cache_insert(XID_STATE *xid_sta
> >  }
> >  
> >  
> > -void xid_cache_delete(XID_STATE *xid_state)
> > +void xid_cache_delete(XID *xid)
> >  {
> >    pthread_mutex_lock(&LOCK_xid_cache);
> > -  hash_delete(&xid_cache, (uchar *)xid_state);
> > +  XID_STATE *xid_state=(XID_STATE *)hash_search(&xid_cache, xid->key(),
> xid->key_length());
> > +  if (xid_state)
> > +    hash_delete(&xid_cache, (uchar *)xid_state);
> >    pthread_mutex_unlock(&LOCK_xid_cache);
> >  }
> >  
> > 
> > === modified file 'sql/sql_class.h'
> > --- a/sql/sql_class.h	2010-05-12 11:19:12 +0000
> > +++ b/sql/sql_class.h	2010-06-18 09:26:33 +0000
> > @@ -753,23 +753,28 @@ struct st_savepoint {
> >  enum xa_states {XA_NOTR=0, XA_ACTIVE, XA_IDLE, XA_PREPARED, XA_ROLLBACK_ONLY};
> >  extern const char *xa_state_names[];
> >  
> > -typedef struct st_xid_state {
> > -  /* For now, this is only used to catch duplicated external xids */
> > -  XID  xid;                           // transaction identifier
> > +typedef struct {
> >    enum xa_states xa_state;            // used by external XA only
> >    bool in_thd;
> >    /* Error reported by the Resource Manager (RM) to the Transaction Manager.
> */
> >    uint rm_error;
> > +} XA_INFO;
> > +
> > +typedef struct st_xid_state {
> > +  /* For now, this is only used to catch duplicated external xids */
> > +  XID  xid;                           // transaction identifier
> > +  XA_INFO value;
> >  } XID_STATE;
> >  
> >  extern pthread_mutex_t LOCK_xid_cache;
> >  extern HASH xid_cache;
> >  bool xid_cache_init(void);
> >  void xid_cache_free(void);
> > -XID_STATE *xid_cache_search(XID *xid);
> > +bool xid_cache_search(XID *xid, XA_INFO* value);
> >  bool xid_cache_insert(XID *xid, enum xa_states xa_state);
> >  bool xid_cache_insert(XID_STATE *xid_state);
> > -void xid_cache_delete(XID_STATE *xid_state);
> > +void xid_cache_delete(XID *xid);
> > +bool xid_cache_find_or_insert(XID *xid, XID_STATE *xid_state, enum xa_states
> xa_state);
> >  
> >  /**
> >    @class Security_context
> > @@ -1515,7 +1520,7 @@ public:
> >          rollback it explicitly, so don't start a new distributed XA until
> >          then.
> >        */
> > -      if (!xid_state.rm_error)
> > +      if (!xid_state.value.rm_error)
> >          xid_state.xid.null();
> >  #ifdef USING_TRANSACTIONS
> >        free_root(&mem_root,MYF(MY_KEEP_PREALLOC));
> > @@ -1528,7 +1533,7 @@ public:
> >        xid_state.xid.null();
> >        init_sql_alloc(&mem_root, ALLOC_ROOT_MIN_BLOCK_SIZE, 0);
> >  #else
> > -      xid_state.xa_state= XA_NOTR;
> > +      xid_state.value.xa_state= XA_NOTR;
> >  #endif
> >      }
> >    } transaction;
> > 
> > === modified file 'sql/sql_parse.cc'
> > --- a/sql/sql_parse.cc	2010-06-01 11:48:24 +0000
> > +++ b/sql/sql_parse.cc	2010-06-18 09:26:33 +0000
> > @@ -97,7 +97,7 @@ const char *xa_state_names[]={
> >    @return TRUE if transaction was rolled back or if the transaction
> >            state is XA_ROLLBACK_ONLY. FALSE otherwise.
> >  */
> > -static bool xa_trans_rolled_back(XID_STATE *xid_state)
> > +static bool xa_trans_rolled_back(const XA_INFO* xid_state)
> >  {
> >    if (xid_state->rm_error)
> >    {
> > @@ -111,7 +111,6 @@ static bool xa_trans_rolled_back(XID_STA
> >      default:
> >        my_error(ER_XA_RBROLLBACK, MYF(0));
> >      }
> > -    xid_state->xa_state= XA_ROLLBACK_ONLY;
> >    }
> >  
> >    return (xid_state->xa_state == XA_ROLLBACK_ONLY);
> > @@ -128,15 +127,15 @@ static bool xa_trans_rollback(THD *thd)
> >      calling ha_rollback(), so thd->transaction.xid structure gets reset
> >      by ha_rollback()/THD::transaction::cleanup().
> >    */
> > -  thd->transaction.xid_state.rm_error= 0;
> > +  thd->transaction.xid_state.value.rm_error= 0;
> >  
> >    bool status= test(ha_rollback(thd));
> >  
> >    thd->options&= ~(ulong) OPTION_BEGIN;
> >    thd->transaction.all.modified_non_trans_table= FALSE;
> >    thd->server_status&= ~SERVER_STATUS_IN_TRANS;
> > -  xid_cache_delete(&thd->transaction.xid_state);
> > -  thd->transaction.xid_state.xa_state= XA_NOTR;
> > +  xid_cache_delete(thd->lex->xid);
> > +  thd->transaction.xid_state.value.xa_state= XA_NOTR;
> >  
> >    return status;
> >  }
> > @@ -161,10 +160,10 @@ bool end_active_trans(THD *thd)
> >      my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
> >      DBUG_RETURN(1);
> >    }
> > -  if (thd->transaction.xid_state.xa_state != XA_NOTR)
> > +  if (thd->transaction.xid_state.value.xa_state != XA_NOTR)
> >    {
> >      my_error(ER_XAER_RMFAIL, MYF(0),
> > -             xa_state_names[thd->transaction.xid_state.xa_state]);
> > +             xa_state_names[thd->transaction.xid_state.value.xa_state]);
> >      DBUG_RETURN(1);
> >    }
> >    if (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN |
> > @@ -722,10 +721,10 @@ int end_trans(THD *thd, enum enum_mysql_
> >      my_error(ER_COMMIT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0));
> >      DBUG_RETURN(1);
> >    }
> > -  if (thd->transaction.xid_state.xa_state != XA_NOTR)
> > +  if (thd->transaction.xid_state.value.xa_state != XA_NOTR)
> >    {
> >      my_error(ER_XAER_RMFAIL, MYF(0),
> > -             xa_state_names[thd->transaction.xid_state.xa_state]);
> > +             xa_state_names[thd->transaction.xid_state.value.xa_state]);
> >      DBUG_RETURN(1);
> >    }
> >    switch (completion) {
> > @@ -4078,10 +4077,10 @@ end_with_restore_list:
> >      break;
> >  
> >    case SQLCOM_BEGIN:
> > -    if (thd->transaction.xid_state.xa_state != XA_NOTR)
> > +    if (thd->transaction.xid_state.value.xa_state != XA_NOTR)
> >      {
> >        my_error(ER_XAER_RMFAIL, MYF(0),
> > -               xa_state_names[thd->transaction.xid_state.xa_state]);
> > +               xa_state_names[thd->transaction.xid_state.value.xa_state]);
> >        break;
> >      }
> >      if (begin_trans(thd))
> > @@ -4704,7 +4703,7 @@ create_sp_error:
> >      break;
> >    }
> >    case SQLCOM_XA_START:
> > -    if (thd->transaction.xid_state.xa_state == XA_IDLE &&
> > +    if (thd->transaction.xid_state.value.xa_state == XA_IDLE &&
> >          thd->lex->xa_opt == XA_RESUME)
> >      {
> >        if (! thd->transaction.xid_state.xid.eq(thd->lex->xid))
> > @@ -4712,7 +4711,7 @@ create_sp_error:
> >          my_error(ER_XAER_NOTA, MYF(0));
> >          break;
> >        }
> > -      thd->transaction.xid_state.xa_state=XA_ACTIVE;
> > +      thd->transaction.xid_state.value.xa_state=XA_ACTIVE;
> >        my_ok(thd);
> >        break;
> >      }
> > @@ -4721,10 +4720,10 @@ create_sp_error:
> >        my_error(ER_XAER_INVAL, MYF(0));
> >        break;
> >      }
> > -    if (thd->transaction.xid_state.xa_state != XA_NOTR)
> > +    if (thd->transaction.xid_state.value.xa_state != XA_NOTR)
> >      {
> >        my_error(ER_XAER_RMFAIL, MYF(0),
> > -               xa_state_names[thd->transaction.xid_state.xa_state]);
> > +               xa_state_names[thd->transaction.xid_state.value.xa_state]);
> >        break;
> >      }
> >      if (thd->active_transaction() || thd->locked_tables)
> > @@ -4732,16 +4731,13 @@ create_sp_error:
> >        my_error(ER_XAER_OUTSIDE, MYF(0));
> >        break;
> >      }
> > -    if (xid_cache_search(thd->lex->xid))
> > +
> > +    if (!xid_cache_find_or_insert(thd->lex->xid,
> &thd->transaction.xid_state, XA_ACTIVE))
> >      {
> >        my_error(ER_XAER_DUPID, MYF(0));
> >        break;
> >      }
> > -    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);
> > +
> >      thd->transaction.all.modified_non_trans_table= FALSE;
> >      thd->options= ((thd->options & ~(OPTION_KEEP_LOG)) |
> OPTION_BEGIN);
> >      thd->server_status|= SERVER_STATUS_IN_TRANS;
> > @@ -4754,10 +4750,10 @@ create_sp_error:
> >        my_error(ER_XAER_INVAL, MYF(0));
> >        break;
> >      }
> > -    if (thd->transaction.xid_state.xa_state != XA_ACTIVE)
> > +    if (thd->transaction.xid_state.value.xa_state != XA_ACTIVE)
> >      {
> >        my_error(ER_XAER_RMFAIL, MYF(0),
> > -               xa_state_names[thd->transaction.xid_state.xa_state]);
> > +               xa_state_names[thd->transaction.xid_state.value.xa_state]);
> >        break;
> >      }
> >      if (!thd->transaction.xid_state.xid.eq(thd->lex->xid))
> > @@ -4765,16 +4761,18 @@ create_sp_error:
> >        my_error(ER_XAER_NOTA, MYF(0));
> >        break;
> >      }
> > -    if (xa_trans_rolled_back(&thd->transaction.xid_state))
> > +    if (xa_trans_rolled_back(&thd->transaction.xid_state.value)) {
> > +      xid_cache_delete(thd->lex->xid);
> >        break;
> > -    thd->transaction.xid_state.xa_state=XA_IDLE;
> > +    }
> > +    thd->transaction.xid_state.value.xa_state=XA_IDLE;
> >      my_ok(thd);
> >      break;
> >    case SQLCOM_XA_PREPARE:
> > -    if (thd->transaction.xid_state.xa_state != XA_IDLE)
> > +    if (thd->transaction.xid_state.value.xa_state != XA_IDLE)
> >      {
> >        my_error(ER_XAER_RMFAIL, MYF(0),
> > -               xa_state_names[thd->transaction.xid_state.xa_state]);
> > +               xa_state_names[thd->transaction.xid_state.value.xa_state]);
> >        break;
> >      }
> >      if (!thd->transaction.xid_state.xid.eq(thd->lex->xid))
> > @@ -4785,39 +4783,40 @@ create_sp_error:
> >      if (ha_prepare(thd))
> >      {
> >        my_error(ER_XA_RBROLLBACK, MYF(0));
> > -      xid_cache_delete(&thd->transaction.xid_state);
> > -      thd->transaction.xid_state.xa_state=XA_NOTR;
> > +      xid_cache_delete(thd->lex->xid);
> > +      thd->transaction.xid_state.value.xa_state=XA_NOTR;
> >        break;
> >      }
> > -    thd->transaction.xid_state.xa_state=XA_PREPARED;
> > +    thd->transaction.xid_state.value.xa_state=XA_PREPARED;
> >      my_ok(thd);
> >      break;
> >    case SQLCOM_XA_COMMIT:
> >      if (!thd->transaction.xid_state.xid.eq(thd->lex->xid))
> >      {
> > -      XID_STATE *xs=xid_cache_search(thd->lex->xid);
> > -      if (!xs || xs->in_thd)
> > +      XA_INFO xid_state;
> > +      bool res=xid_cache_search(thd->lex->xid, &xid_state);
> > +      if (!res || xid_state.in_thd)
> >          my_error(ER_XAER_NOTA, MYF(0));
> > -      else if (xa_trans_rolled_back(xs))
> > +      else if (xa_trans_rolled_back(&xid_state))
> >        {
> >          ha_commit_or_rollback_by_xid(thd->lex->xid, 0);
> > -        xid_cache_delete(xs);
> > +        xid_cache_delete(thd->lex->xid);
> >          break;
> >        }
> >        else
> >        {
> >          ha_commit_or_rollback_by_xid(thd->lex->xid, 1);
> > -        xid_cache_delete(xs);
> > +        xid_cache_delete(thd->lex->xid);
> >          my_ok(thd);
> >        }
> >        break;
> >      }
> > -    if (xa_trans_rolled_back(&thd->transaction.xid_state))
> > +    if (xa_trans_rolled_back(&thd->transaction.xid_state.value))
> >      {
> >        xa_trans_rollback(thd);
> >        break;
> >      }
> > -    if (thd->transaction.xid_state.xa_state == XA_IDLE &&
> > +    if (thd->transaction.xid_state.value.xa_state == XA_IDLE &&
> >          thd->lex->xa_opt == XA_ONE_PHASE)
> >      {
> >        int r;
> > @@ -4826,7 +4825,7 @@ create_sp_error:
> >        else
> >          my_ok(thd);
> >      }
> > -    else if (thd->transaction.xid_state.xa_state == XA_PREPARED &&
> > +    else if (thd->transaction.xid_state.value.xa_state == XA_PREPARED
> &&
> >               thd->lex->xa_opt == XA_NONE)
> >      {
> >        if (wait_if_global_read_lock(thd, 0, 0))
> > @@ -4846,37 +4845,39 @@ create_sp_error:
> >      else
> >      {
> >        my_error(ER_XAER_RMFAIL, MYF(0),
> > -               xa_state_names[thd->transaction.xid_state.xa_state]);
> > +               xa_state_names[thd->transaction.xid_state.value.xa_state]);
> >        break;
> >      }
> >      thd->options&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
> >      thd->transaction.all.modified_non_trans_table= FALSE;
> >      thd->server_status&= ~SERVER_STATUS_IN_TRANS;
> > -    xid_cache_delete(&thd->transaction.xid_state);
> > -    thd->transaction.xid_state.xa_state=XA_NOTR;
> > +    xid_cache_delete(thd->lex->xid);
> > +    thd->transaction.xid_state.value.xa_state=XA_NOTR;
> >      break;
> >    case SQLCOM_XA_ROLLBACK:
> >      if (!thd->transaction.xid_state.xid.eq(thd->lex->xid))
> >      {
> > -      XID_STATE *xs=xid_cache_search(thd->lex->xid);
> > -      if (!xs || xs->in_thd)
> > +      XA_INFO xid_state;
> > +      bool res=xid_cache_search(thd->lex->xid, &xid_state);
> > +
> > +      if (!res || xid_state.in_thd)
> >          my_error(ER_XAER_NOTA, MYF(0));
> >        else
> >        {
> > -        bool ok= !xa_trans_rolled_back(xs);
> > +        bool ok= !xa_trans_rolled_back(&xid_state);
> >          ha_commit_or_rollback_by_xid(thd->lex->xid, 0);
> > -        xid_cache_delete(xs);
> > +        xid_cache_delete(thd->lex->xid);
> >          if (ok)
> >            my_ok(thd);
> >        }
> >        break;
> >      }
> > -    if (thd->transaction.xid_state.xa_state != XA_IDLE &&
> > -        thd->transaction.xid_state.xa_state != XA_PREPARED &&
> > -        thd->transaction.xid_state.xa_state != XA_ROLLBACK_ONLY)
> > +    if (thd->transaction.xid_state.value.xa_state != XA_IDLE &&
> > +        thd->transaction.xid_state.value.xa_state != XA_PREPARED &&
> > +        thd->transaction.xid_state.value.xa_state != XA_ROLLBACK_ONLY)
> >      {
> >        my_error(ER_XAER_RMFAIL, MYF(0),
> > -               xa_state_names[thd->transaction.xid_state.xa_state]);
> > +               xa_state_names[thd->transaction.xid_state.value.xa_state]);
> >        break;
> >      }
> >      if (xa_trans_rollback(thd))
> > 
> 
> > # Bazaar merge directive format 2 (Bazaar 0.90)
> > # revision_id: dmitry.shulga@stripped
> > # target_branch: file:///Users/shulga/projects/mysql/5.1-bug51855/
> > # testament_sha1: cdd98be9c185a81f1984a55bb5d639a3dea08210
> > # timestamp: 2010-06-18 16:26:38 +0700
> > # base_revision_id: build@stripped
> > # 
> > # Begin bundle
> > IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWUia1B4ACR7/gFQ6AJB59///
> > f+//ar////5gEBzuvnYrh7uvKaeL7C7CS+sPj1fNtfWKoro5d7NexuzD2Y9mC1rbQkpNT0qewjKm
> > 9TYqbymjCekZIAwAIwQYRgTQCSITJhTCIp4jUeppk0NBoA0AAAAAGg00CCk9CR6mmxR6jTQ0yAaA
> > DQAAAAAJCkE9SaKfppT9NGSeh6lN6k9EbU8UzQj1AAAD1NBtQIpIAVT/TQmAp5DSh4mmmpmjUNGI
> > NNPUGgAGagiiICaEZDEyBMUbIxSepp+qNANA09R6gAGhVJfUxIP+4dPNj5cNeBs03ptjbZjijb4H
> > G6uJtMY6EIxu0JO9o/b8PrJtfz9+wvxGbHtzExZncmQqGVFC7kOHMqKFq0YHKFATlv9udesndULO
> > DN33drXgdUPPkP5Dmxu2M2nmuWj8c2SyYEBY6YbW6ocqWGhZq5osRQvChJLiZjeyWra8Mz0C0o8m
> > 91pOGMW1xuwzUkv01u4KANoK0gE+EBrU8/l9JZEb2f0Uj5D0f44sKg3kJfbuwGgZtHkn8E3JANpN
> > iSGwbHtdzP1SEbmji06bVvVvshazVxS1gKdBuXaJcRdXatwBERKqOM6rAtWbOjTZ4Cm5hlAsVLEB
> > WiKPdle1INDBYsWLFilSjFjQ3KmhgpQo5JqRJU1Kl6WvLxJJiL0cJDarMTtcZVlY8dDE2zZnN7wN
> > ehYeLCg7IezfM+/rIGJGOhw3MhEDuDq28WF/Mly+6hAntYILnkSJ28uJbXtks2OWfgYg+o7WjY2p
> > kiGzv+s8eKTy2RAZisB7SIV/yrrQuEu8jHLWVz9+KrGiJamocaccOWnNNrTMW988BLk3E6PQtTIq
> > I0wSvLSyimB6Z+jTgp45U6aw7ZZRnOAv4KcAtzlh24yI8/RGI0t+HjNDLhnZtJBpwMSGJjDBkx2W
> > X8s0M2IHZkRZEQ+EMbtMQ+wSMvWho3U2A2+8OhQR7qM5oylDtnC8DRLi8gow1r3IEokSGAAU75fa
> > +HWOhyo1a4ZQfq8F99hvdJyW1tI91GST1kayNjkhCDdsjjmp1psD3j5nVllMx6/VKF+LDAfGJI+E
> > mgsO7w8Hgwh6ClfYnhmQHDbmgiO8ST1yfNxF+5222Pu4pfNhvfDQiIKK02dnGGDZOXWa+XNm3M+a
> > 8aiiTcsB8Cgv77bQ7otA1/RKzYLPpCCF4IZHf3dn1wnMW4XpphzM5kho0efk1x0ZgZjhMvXAz7WU
> > K1kR3SKlAKEOUA1LZiq1KAMQKyzDTRxNQEcVaikRFbCRTEGoOMYFDGGt1iMtWl5dNvUQyYyCUJts
> > YtgwiAobBb1a1B9fqouTo8LewYPFUmaU/qIlKgITtc0hMi0O7CPByQkOwLhNwXOc20N6FOFE06am
> > Rzd7SGPXxyucoadFNGX1sMbRIQanQUhPlGuSbqSlQqz/3y3OJmVtdQTcBsT4EFyoWtI17N4vrQ+e
> > 0y4wv07wdxn462mPRzJiRypYbaSZXOYl1RhorqoomhS/RizE0SosMVBlbSBSii0HpWudM6sp5bdF
> > ISKQqRjltesbiWrjXraNWWIZJzlNBsQ1ax6Y5rJuZd9XRp2aKvNisVs0Y7RhisN5V3hZJXzrQoWF
> > MFG0mWlYSVTNOpzZUhkSk4lFGTJXGHYZjlSERtRlO+B6xJmqdxg462lNFDMYTE9ZPGInkmx7hZ8h
> > QbiWNMTudKbXuciKmxjxZWReIzDG5ZXXZ4WSI2pjNBKGwCgCErVhJN3bTYrCPl20My4ks1RxFGQD
> > 3IHYHm65i1ka6jLUqZSKnhkf8Big2dnWt97YGjkzO7Thu8zBv2xp/Z0c3SXyrPniWjkaZBpG8Fjx
> > o1xS2ZPbzN4z7anSLXYrF+iddSN8qAqHY0MiUGMzQ7jmQxC51qacpK4C5NticZDqQwzO7KUAF2ig
> > zrW0sKUwFwiYDTazTILBhli7xr4JGEzJgVFxRhAlRcaz9WFxxqlYuS7vEScqPE3NSll0o1XJch96
> > MQMr6yw7wg0tEz3BYHGOZyJDJVdmZm3XYjLBImF4tmBqyqDIKrjZsa7LWYWa+UVuJxAyChqFE4qM
> > DzFnIjw2p/ZcfApcQkxVnSlBivKBaCxr5KOcpqL1Glmec7Kozu8adNjKMaAwERRuBEtEAZAblHfi
> > ijlK5GREjCQ1BhUVXXktyz69bE+rF+vcqTGQ5PGkhLu55Y4WjF3i2c3lnOWZTiAyYUOChA6ZywWE
> > pZSJkYMCvYgCp2u1BTjsQxtdtCVbmDwu8ndgrLgrEFCMXgi2zvS8BQFN0ooemUIMKhN2ZfG6gXWV
> > mL4av0jN4pDrabs+KKMLWXP00ibvhE2m/xTfh3qeV6ubq4egXTOnEdsGBtkhYWZa3IgVRkoMQwZc
> > ERia2zj86O7yAgPFVeXzV65qBfaB0Pc/4nmPTNJShENZbBEQMH9u/k7XkfVw7TvLV/V5/cFD6YAd
> > XdeufdA76OJzdsDI0ezIDZlo91Esy5EhSAeFvZ/9G99xpTS+xwlYwmjODOQn2g4utyGuAl6Z6BxG
> > kTam9LLClsUtme2ZLFI3/blxQQXo3KZ3iE6bHbjBlEh8+eQjF6xgoMZSH5JMOIcZn50GtB7S72CX
> > bA7lNp31Q9InhrfJCE34qSH1wlHWkk8lUpJAFh/HBohpkcUkbqwQt4nmLaEQkQEQokAQQRALaDCV
> > k6YMfceOBqhLAXMrZ9LXLzqBcI3kOUjSV0El8zmt/e/TmpekSAkuobqRDU8G87zMZirUWzpWNgFU
> > bLfvqa9YecFaRdd+oZ3jQRIxi1OMNWkTiojgQlspnwoFyOmyLMytrEQgDMNyVmK2BleHI+OW8j9Z
> > oPsOlKmmpX7AuLo9qc3s6TadJmoSdmuouU73/U+4g6Q4fweTY0aPVO+G4Nbux72wvBApyAGU2afW
> > XI92HOIFJTDZw0C+oBBAcRjGw5g4iflL8oz99yFUVX4rCZQM70SuCEplsqcolFuApropg1wVve3O
> > cMEQm7wq6q1DTCSEyRCToJmt9YIQKTIQimtJJHft3byNhuNxf2nZKWmS59OkJzrXubgdJc3JQQNb
> > aMqLuzFLLLzIlINBmxFJMWI21Zyw7dctr3x2OC+UqGEq0bHtvvxyx4qFvWDrhnRNNbIbjj27Umt7
> > cWxroV+Nj5W1RhgxpurZJBpjCt6HZQYKQEEBA5SBpkeh4kPUaydD3nytvivvu6oOs9hvO4vMxQzm
> > jLTxkMzJ5p6JKHJjdDSCZQoIcgkcgFEJA1sRzHvchOc1kE+OFJtnqxpWlLecLbXNkmPQSuk1BlGV
> > 6ZzrjLKLpAZjTRUUpxrrDpSTMk8XvVbQvt7bCur0MwSKisqGiiZoqG01L+tg1lghB9ksESIsUj7t
> > JPFPB8gaIdsZjnbRldSGeFnRaOhGgEwBIlLkhSNxTBnrsAb/nnJTcaviN5zlJxmZkukHJKWyiSO5
> > ma+qxQsN3GLCnRXXgOAuhDFW+1Hp8EhKpwHA8s6dBYNsz1NMz6PdaRFHzeDWvWAahZvx5zz19rsT
> > zvLTyTpO73s4yUISWD5uiqczTqppIgXr6nrncvVw+z8p/K73BBawMkKOuWE+SHl9/z0CH6zkFue8
> > VbE8+UWTR3hvvYCvZOfyO8BV4ERpDQkjfdOQK2HUanInIRjYxs4QJ77+Z5WxEigqDl2MuNkktgFQ
> > 6cJ67sdJiyAU9PRw3WjlgEIiB8ZVGHOBNu13kdCB2nvtqcGXbPrdDS9AuPhewWpSvpvTsC7YBO1Z
> > 9rzttocWqRmao9wejqArR7bADtwF1Pe8dl3XxoXZ5VWRudpE0LkIqAY655keziiBDlz9eGlRrjMM
> > VYp8JwV5dbvs6mwxCnRqUyuaQXJsqMcyGkchQ+c1C496YOEJiG20b0vaHO+B0c3qbbsjGW4kkk0h
> > pmUIUSkQDSxMUNnQ3hpuq3lBwqLNCEhJGykHcULCzHGsN6er4ti4iJahdijMIZQdDRyeDUOXCiMc
> > KS6XYmmhyBt62w6rNzyngxrEqnWqdOtQZDLruCnKJcccBqvJ5KgVc4FDgmaqJWqJ1ajfyTg8p1W2
> > 8CPiJcbmxhvkkLwJCVD8LlwTmcb1tvHxOjWzXJdivjnjxXPS6U0t+fS/A6hWl7HKmDwPLqnhJQHD
> > KRE4QxEMiccyhGywROm1OjMyJR2kM1DBvaDuhabI+B4SJSDn98jncNOBNA6uJ7H6G/zXANaaxx4+
> > lKFDmY5tGgZuU3N1oPEkCZr1nxZo20k8OfjuPAsv6LHWl1L704/HFQCr5E8YikBXWWCJQPio6m22
> > EyJgSODTAEZZ8cwbaIL0JtXl0Oye9Kp4StKheRpzATpe5m+Aq53lxJij3pSlMzMzMznTgylKNZlm
> > 6lGlmlHlHudXHjWeIWIEBiDsqBQK+4Qp4seHB3Zc22uevXusyjfbhSg1r4ZnrbMbEoJambmYAVsB
> > pDELCkCoVhQtjfSTQuoTPo9CxOUPaJb8WPpoB+JhpT3RB/5C4DfY+DHmmbnjXs3muuqN8kAhFlzg
> > jtAmbr0n7GIVc1zqCqsKW2aFYxQsr01gUZB1Rs3yoda0qNJtIE+PoS0KzPgAZejNDWNdkwhTqIcJ
> > o0IbODoacAkzybkmb3pVpP3rTwEdHixQliYVowTCQQhM6U4XBqdnDWwBwJodDSOUurY4nN5nN4wv
> > RyC1DD6yFD293cMqDBdk5q+E0LVpDJWKjTDEEHpWwIc7PH5FSURuMdRDdGP4MCIM2UYlkWlXCEDG
> > FaFsbw9Pa34RjcoO53RTIiUxMRCvmdD3p3lneSt4nmDldrYEhfY6O1sHxe98r9Po143S7U9v+ySY
> > 2RP/xdyRThQkEia1B4A=
> > 
> > 
> 
> > 
> > -- 
> > MySQL Code Commits Mailing List
> > For list archives: http://lists.mysql.com/commits
> > To unsubscribe:    http://lists.mysql.com/commits?unsub=1
> 
> 
> -- 
> Sergey Vojtovich <svoj@stripped>
> MySQL AB, Software Engineer
> Izhevsk, Russia, www.mysql.com
> 
> -- 
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:    http://lists.mysql.com/commits?unsub=1
> 

-- 
Sergey Vojtovich <svoj@stripped>
MySQL AB, Software Engineer
Izhevsk, Russia, www.mysql.com
Thread
bzr commit into mysql-5.1 branch (Dmitry.Shulga:3411) Bug#51855Dmitry Shulga18 Jun
  • Re: bzr commit into mysql-5.1 branch (Dmitry.Shulga:3411) Bug#51855Sergey Vojtovich19 Jun
    • Re: bzr commit into mysql-5.1 branch (Dmitry.Shulga:3411) Bug#51855Sergey Vojtovich21 Jun