Skip to content

Commit dc11654

Browse files
committed
Do not use MEM_ROOT in set_killed_no_mutex()
The reason for this change are the following: - If we call set_killed() from one thread to kill another thread with a message, there may be concurrent usage of the MEM_ROOT which is not supported (this could cause memory corruption). We do not currently have code that does this, but the API allows this and it is better to be fix the issue before it happens. - The per thread memory tracking does not work if one thread uses another threads MEM_ROOT. - set_killed() can be called if a MEM_ROOT allocation fails. In this case it is not good to try to allocate more memory from potentially the same MEM_ROOT. Fix is to use my_malloc() instead of mem_root for killed messages.
1 parent 9e424b6 commit dc11654

File tree

3 files changed

+14
-2
lines changed

3 files changed

+14
-2
lines changed

sql/sql_class.cc

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1649,6 +1649,7 @@ void THD::reset_for_reuse()
16491649
wsrep_cs().reset_error();
16501650
wsrep_aborter= 0;
16511651
wsrep_abort_by_kill= NOT_KILLED;
1652+
my_free(wsrep_abort_by_kill_err);
16521653
wsrep_abort_by_kill_err= 0;
16531654
#ifndef DBUG_OFF
16541655
wsrep_killed_state= 0;
@@ -1689,6 +1690,8 @@ THD::~THD()
16891690

16901691
#ifdef WITH_WSREP
16911692
mysql_cond_destroy(&COND_wsrep_thd);
1693+
my_free(wsrep_abort_by_kill_err);
1694+
wsrep_abort_by_kill_err= 0;
16921695
#endif
16931696
mdl_context.destroy();
16941697

@@ -1708,6 +1711,7 @@ THD::~THD()
17081711
main_lex.free_set_stmt_mem_root();
17091712
free_root(&main_mem_root, MYF(0));
17101713
my_free(m_token_array);
1714+
my_free(killed_err);
17111715
main_da.free_memory();
17121716
if (tdc_hash_pins)
17131717
lf_hash_put_pins(tdc_hash_pins);
@@ -2131,7 +2135,11 @@ void THD::reset_killed()
21312135
mysql_mutex_assert_not_owner(&LOCK_thd_kill);
21322136
mysql_mutex_lock(&LOCK_thd_kill);
21332137
killed= NOT_KILLED;
2134-
killed_err= 0;
2138+
if (unlikely(killed_err))
2139+
{
2140+
my_free(killed_err);
2141+
killed_err= 0;
2142+
}
21352143
mysql_mutex_unlock(&LOCK_thd_kill);
21362144
}
21372145
#ifdef WITH_WSREP
@@ -2142,6 +2150,7 @@ void THD::reset_killed()
21422150
mysql_mutex_assert_not_owner(&LOCK_thd_kill);
21432151
mysql_mutex_lock(&LOCK_thd_kill);
21442152
wsrep_abort_by_kill= NOT_KILLED;
2153+
my_free(wsrep_abort_by_kill_err);
21452154
wsrep_abort_by_kill_err= 0;
21462155
mysql_mutex_unlock(&LOCK_thd_kill);
21472156
}

sql/sql_class.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4182,7 +4182,8 @@ class THD: public THD_count, /* this must be first */
41824182
The worst things that can happen is that we get
41834183
a suboptimal error message.
41844184
*/
4185-
killed_err= (err_info*) alloc_root(&main_mem_root, sizeof(*killed_err));
4185+
if (!killed_err)
4186+
killed_err= (err_info*) my_malloc(sizeof(*killed_err), MYF(MY_WME));
41864187
if (likely(killed_err))
41874188
{
41884189
killed_err->no= killed_errno_arg;

sql/wsrep_thd.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,7 @@ void wsrep_backup_kill_for_commit(THD *thd)
485485
thd->wsrep_trx().state() != wsrep::transaction::s_must_replay)
486486
{
487487
thd->wsrep_abort_by_kill= thd->killed;
488+
my_free(thd->wsrep_abort_by_kill_err);
488489
thd->wsrep_abort_by_kill_err= thd->killed_err;
489490
thd->killed= NOT_KILLED;
490491
thd->killed_err= 0;
@@ -497,6 +498,7 @@ void wsrep_restore_kill_after_commit(THD *thd)
497498
DBUG_ASSERT(WSREP(thd));
498499
mysql_mutex_assert_owner(&thd->LOCK_thd_kill);
499500
thd->killed= thd->wsrep_abort_by_kill;
501+
my_free(thd->killed_err);
500502
thd->killed_err= thd->wsrep_abort_by_kill_err;
501503
thd->wsrep_abort_by_kill= NOT_KILLED;
502504
thd->wsrep_abort_by_kill_err= 0;

0 commit comments

Comments
 (0)