Skip to content

Commit 1924d8f

Browse files
GlebShchepadahlerlend
authored andcommitted
Bug#33728209: set variable to function value cause double locking mutex & deadlock/hang
Assignment to the @@session_track_system_variables variable caused a recursive locking of the LOCK_plugin mutex in corner cases. The fix removes an ad-hoc processing of the @@session_track_system_variables from the System_variable_tracker class. Change-Id: I38748f3a6f767ef7023ea40dbfd0c46322c864bf (cherry picked from commit e5b2f18f360ef1ea99ef75a5033d1f4e72c7c088) (cherry picked from commit 053b58cf5c7459aeb47019974ede11609fa54b48)
1 parent c544cce commit 1924d8f

File tree

4 files changed

+33
-46
lines changed

4 files changed

+33
-46
lines changed

mysql-test/r/session_tracker.result

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -843,5 +843,19 @@ session_track_state_change ON
843843
SHOW VARIABLES like 'session_track_state_change';
844844
Variable_name Value
845845
session_track_state_change OFF
846+
#
847+
# Bug#33728209: set variable to function value cause double locking mutex & deadlock/hang
848+
#
849+
create function f() returns longtext no sql
850+
begin
851+
declare continue handler for sqlexception begin end;
852+
replace into asd values();
853+
return 'a';
854+
end $
855+
set session_track_system_variables=f();
856+
Warnings:
857+
Warning 1231 a is not a valid system variable and will be ignored.
858+
set session_track_system_variables=DEFAULT;
859+
drop function f;
846860

847861
# End of tests

mysql-test/t/session_tracker.test

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,5 +532,23 @@ SHOW VARIABLES like 'session_track_state_change';
532532

533533
connection default;
534534

535+
--echo #
536+
--echo # Bug#33728209: set variable to function value cause double locking mutex & deadlock/hang
537+
--echo #
538+
539+
delimiter $;
540+
create function f() returns longtext no sql
541+
begin
542+
declare continue handler for sqlexception begin end;
543+
replace into asd values();
544+
return 'a';
545+
end $
546+
delimiter ;$
547+
548+
set session_track_system_variables=f();
549+
set session_track_system_variables=DEFAULT;
550+
551+
drop function f;
552+
535553
--echo
536554
--echo # End of tests

sql/set_var.cc

Lines changed: 1 addition & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -768,11 +768,7 @@ bool System_variable_tracker::access_system_variable(
768768
thd, suppress_not_found_error == Suppress_not_found_error::YES))
769769
return true;
770770
if (f) {
771-
extern sys_var *Sys_track_session_sys_vars_ptr;
772-
if (m_static.m_static_var == Sys_track_session_sys_vars_ptr)
773-
visit_session_track_system_variables(thd, f);
774-
else
775-
f(*this, m_static.m_static_var);
771+
f(*this, m_static.m_static_var);
776772
}
777773
return false;
778774
case KEYCACHE:
@@ -1132,43 +1128,6 @@ bool System_variable_tracker::enumerate_sys_vars(
11321128
return false;
11331129
}
11341130

1135-
void System_variable_tracker::visit_session_track_system_variables(
1136-
THD *thd,
1137-
std::function<void(const System_variable_tracker &, sys_var *)> f) const {
1138-
assert(m_tag == STATIC);
1139-
extern sys_var *Sys_track_session_sys_vars_ptr;
1140-
if (m_static.m_static_var != Sys_track_session_sys_vars_ptr)
1141-
assert(false); // should never happen
1142-
if (f == nullptr) {
1143-
return;
1144-
}
1145-
if (thd != nullptr) {
1146-
if (thd->plugin_lock_recursion_depth++ == 0) {
1147-
/*
1148-
Historically, we acquire LOCK_plugin *before*
1149-
LOCK_system_variables_hash, so, we are not allowed to "upgrade" an
1150-
existent LOCK_system_variables_hash
1151-
to LOCK_plugin. Thus, even if LOCK_plugin is not needed at some point
1152-
where we're acquiring LOCK_system_variables_hash, we still have to
1153-
hold LOCK_plugin because of possible indirect visit_plugin_variable()
1154-
call. So, let lock LOCK_plugin "in advance":
1155-
*/
1156-
mysql_mutex_lock(&LOCK_plugin);
1157-
}
1158-
if (thd->system_variable_hash_lock_recursion_depth++ == 0)
1159-
mysql_rwlock_rdlock(&LOCK_system_variables_hash);
1160-
}
1161-
1162-
f(*this, m_static.m_static_var);
1163-
1164-
if (thd != nullptr) {
1165-
if (--thd->system_variable_hash_lock_recursion_depth == 0)
1166-
mysql_rwlock_unlock(&LOCK_system_variables_hash);
1167-
if (--thd->plugin_lock_recursion_depth == 0)
1168-
mysql_mutex_unlock(&LOCK_plugin);
1169-
}
1170-
}
1171-
11721131
bool System_variable_tracker::visit_plugin_variable(
11731132
THD *thd, std::function<bool(sys_var *)> function,
11741133
Suppress_not_found_error suppress_not_found_error) const {

sql/set_var.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -851,10 +851,6 @@ class System_variable_tracker final {
851851
}
852852

853853
private:
854-
void visit_session_track_system_variables(
855-
THD *,
856-
std::function<void(const System_variable_tracker &, sys_var *)>) const;
857-
858854
bool visit_plugin_variable(THD *, std::function<bool(sys_var *)>,
859855
Suppress_not_found_error) const;
860856

0 commit comments

Comments
 (0)