Hi, Marko! On Dec 19, Marko Mäkelä wrote:
I have already approved the InnoDB changes.
Yes, I've noticed :)
On Sat, Dec 19, 2020 at 12:43 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Jan! [snip]
+ mysql_mutex_lock(&LOCK_thd_kill); + mysql_mutex_unlock(&LOCK_thd_kill);
Note that THD destructor starts from `assert_not_linked()` - it's important, because if the THD is linked in the list of threads, then someone might start using it after you released LOCK_thd_kill.
With the new logic you need to move assert_not_linked to free_connection().
otherwise ok to push
Do you mean that also the empty mutex lock/unlock pair should be removed from THD::~THD()? It was copied (not moved) from there in this patch.
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. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org