← Back to team overview

maria-developers team mailing list archive

Re: 889f90365ef: MDEV-23536 : Race condition between KILL and transaction commit


Hi Sergei,

> Not exactly. In 10.2 THD::~THD() only has
>   mysql_mutex_lock(&LOCK_thd_data);
>   mysql_mutex_unlock(&LOCK_thd_data);
> it's not what Jan did. So I thought it'd be safer to leave it in place.
> But 10.5 has
>   if (WSREP_NNULL(this)) mysql_mutex_lock(&LOCK_thd_data);
>   mysql_mutex_lock(&LOCK_thd_kill);
>   mysql_mutex_unlock(&LOCK_thd_kill);
>   if (WSREP_NNULL(this)) mysql_mutex_unlock(&LOCK_thd_data);
> which is *exactly* what Jan did, so yes, when merging this chunk should
> go away (and it's quite likely that you'll be doing this merge, so
> please don't forget). Because of Jan's change in THD::~THD() there will
> be a merge conflict in the destructor, so it cannot slip unnoticed and
> auto-merged.

I do not follow merging this chunk should go away here.

10.2 :
10.3 :
10.6 :

Looking into KILL:
10.2: KILL find_thread_by_id acquires LOCK_THD_data and THD::set_killed
aquires LOCK_thd_kill
10.3-10.6: KILL find_thread_by_id acquires LOCK_thd_data (wsrep) and
                 THD::set_killed acquires LOCK_thd_kill

In all cases I do:
void THD::free_connection()
  DBUG_ASSERT(free_connection_done == 0);
    wsrep_rgi should be protected with LOCK_thd_data mutex and
    mutexing ordering rules state that LOCK_thd_data must be taken
    before LOCK_thd_kill.
  if (wsrep_rgi) mysql_mutex_lock(&LOCK_thd_data);
    Other threads may have a lock on LOCK_thd_kill to ensure that this
    THD is not deleted while they access it. The following mutex_lock
    ensures that no one else is using this THD and it's now safe to

  if (rpl_group_info *rgi= wsrep_rgi)
    wsrep_rgi= 0;
    delete rgi;

Code looks like that because Marko requested not to do delete rgi under a
So do you mean that I should take LOCK_thd_data even when we do not have
wsrep_rgi for wsrep as if I correctly understand
wsrep_rgi is there only for applier not all THDs with WSREP(thd).

R: Jan

Follow ups