Skip to content

Commit 0aedcb1

Browse files
percona-ysorokinpercona-ysorokin
authored
Merge pull request #5582 from percona-ysorokin/dev/PS-9723-8.0-reload_tls_x_plugin_hook_race
PS-9723 fix: Crash in xpl::Ssl_context::~Ssl_context() with heavy load of ALTER INSTANCE RELOAD TLS queries (8.0)
2 parents 3f1fccb + b5c8673 commit 0aedcb1

File tree

7 files changed

+131
-10
lines changed

7 files changed

+131
-10
lines changed

‎mysql-test/suite/x/r/percona_bug_ps9723.result‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
#
2+
# PS-9723: Crash in xpl::Ssl_context::~Ssl_context() with heavy load of ALTER INSTANCE RELOAD TLS queries
3+
# (https://perconadev.atlassian.net/browse/PS-9723)
4+
#
5+
CREATE PROCEDURE reload_tls_loop(IN iterations INT UNSIGNED)
6+
BEGIN
7+
DECLARE idx INT UNSIGNED DEFAULT iterations;
8+
WHILE idx > 0 DO
9+
ALTER INSTANCE RELOAD TLS NO ROLLBACK ON ERROR;
10+
SET idx = idx - 1;
11+
END WHILE;
12+
END//
13+
# Establising 32 connections
14+
# Calling reload_tls_loop(1024) from each connection
15+
CALL reload_tls_loop(1024);
16+
CALL reload_tls_loop(1024);
17+
CALL reload_tls_loop(1024);
18+
CALL reload_tls_loop(1024);
19+
CALL reload_tls_loop(1024);
20+
CALL reload_tls_loop(1024);
21+
CALL reload_tls_loop(1024);
22+
CALL reload_tls_loop(1024);
23+
CALL reload_tls_loop(1024);
24+
CALL reload_tls_loop(1024);
25+
CALL reload_tls_loop(1024);
26+
CALL reload_tls_loop(1024);
27+
CALL reload_tls_loop(1024);
28+
CALL reload_tls_loop(1024);
29+
CALL reload_tls_loop(1024);
30+
CALL reload_tls_loop(1024);
31+
CALL reload_tls_loop(1024);
32+
CALL reload_tls_loop(1024);
33+
CALL reload_tls_loop(1024);
34+
CALL reload_tls_loop(1024);
35+
CALL reload_tls_loop(1024);
36+
CALL reload_tls_loop(1024);
37+
CALL reload_tls_loop(1024);
38+
CALL reload_tls_loop(1024);
39+
CALL reload_tls_loop(1024);
40+
CALL reload_tls_loop(1024);
41+
CALL reload_tls_loop(1024);
42+
CALL reload_tls_loop(1024);
43+
CALL reload_tls_loop(1024);
44+
CALL reload_tls_loop(1024);
45+
CALL reload_tls_loop(1024);
46+
CALL reload_tls_loop(1024);
47+
# Waiting for reload_tls_loop(1024) to finish in each connection
48+
# Closing the connections
49+
DROP PROCEDURE reload_tls_loop;

‎mysql-test/suite/x/t/percona_bug_ps9723.test‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
--echo #
2+
--echo # PS-9723: Crash in xpl::Ssl_context::~Ssl_context() with heavy load of ALTER INSTANCE RELOAD TLS queries
3+
--echo # (https://perconadev.atlassian.net/browse/PS-9723)
4+
--echo #
5+
6+
--source include/have_mysqlx_plugin.inc
7+
--source include/big_test.inc
8+
9+
--source include/count_sessions.inc
10+
11+
DELIMITER //;
12+
13+
CREATE PROCEDURE reload_tls_loop(IN iterations INT UNSIGNED)
14+
BEGIN
15+
DECLARE idx INT UNSIGNED DEFAULT iterations;
16+
17+
WHILE idx > 0 DO
18+
ALTER INSTANCE RELOAD TLS NO ROLLBACK ON ERROR;
19+
SET idx = idx - 1;
20+
END WHILE;
21+
END//
22+
23+
DELIMITER ;//
24+
25+
--let $number_of_connections = 32
26+
--let $number_of_iterations = 1024
27+
28+
--echo # Establising $number_of_connections connections
29+
--let $i = 0
30+
while ($i < $number_of_connections)
31+
{
32+
--connect(con$i,127.0.0.1,root,,,,,SSL)
33+
--inc $i
34+
}
35+
36+
--echo # Calling reload_tls_loop($number_of_iterations) from each connection
37+
--let $i = 0
38+
while ($i < $number_of_connections)
39+
{
40+
--connection con$i
41+
--send_eval CALL reload_tls_loop($number_of_iterations)
42+
--inc $i
43+
}
44+
45+
--echo # Waiting for reload_tls_loop($number_of_iterations) to finish in each connection
46+
--let $i = 0
47+
while ($i < $number_of_connections)
48+
{
49+
--connection con$i
50+
--reap
51+
--inc $i
52+
}
53+
54+
--echo # Closing the connections
55+
--let $i = 0
56+
while ($i < $number_of_connections)
57+
{
58+
--disconnect con$i
59+
--inc $i
60+
}
61+
--connection default
62+
63+
--source include/wait_until_count_sessions.inc
64+
65+
DROP PROCEDURE reload_tls_loop;

‎plugin/x/src/server/builder/ssl_context_builder.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,9 @@ void Ssl_context_builder::setup_ssl_context(
114114
}
115115
}
116116

117-
std::unique_ptr<iface::Ssl_context> Ssl_context_builder::get_result_context()
117+
std::shared_ptr<iface::Ssl_context> Ssl_context_builder::get_result_context()
118118
const {
119-
std::unique_ptr<iface::Ssl_context> result = std::make_unique<Ssl_context>();
119+
std::shared_ptr<iface::Ssl_context> result = std::make_shared<Ssl_context>();
120120

121121
setup_ssl_context(result.get());
122122

‎plugin/x/src/server/builder/ssl_context_builder.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class Ssl_context_builder {
3838
public:
3939
Ssl_context_builder() = default;
4040

41-
std::unique_ptr<iface::Ssl_context> get_result_context() const;
41+
std::shared_ptr<iface::Ssl_context> get_result_context() const;
4242

4343
private:
4444
struct Ssl_config_local {

‎plugin/x/src/server/server.cc‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,13 +136,15 @@ void Server::delayed_start_tasks() {
136136
}
137137

138138
void Server::reload_ssl_context() {
139-
m_ssl_context = xpl::Ssl_context_builder().get_result_context();
139+
std::atomic_store(&m_ssl_context,
140+
xpl::Ssl_context_builder().get_result_context());
140141
}
141142

142143
void Server::start_tasks() {
143144
// We can't fetch the servers ssl config at plugin-load
144145
// this method allows to setup it at better time.
145-
m_ssl_context = xpl::Ssl_context_builder().get_result_context();
146+
std::atomic_store(&m_ssl_context,
147+
xpl::Ssl_context_builder().get_result_context());
146148

147149
if (m_state.exchange(State::State_initializing, State_running)) {
148150
for (auto task : m_tasks) {
@@ -436,7 +438,8 @@ bool Server::reset() {
436438

437439
m_state.wait_for(allowed_values);
438440

439-
m_ssl_context->reset();
441+
auto context = std::atomic_load(&m_ssl_context);
442+
context->reset();
440443
m_id_generator.reset(new Document_id_generator());
441444
m_factory.reset(new xpl::Server_factory());
442445

‎plugin/x/src/server/server.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
#include <stdint.h>
3030

31+
#include <atomic>
3132
#include <cstdint>
3233
#include <functional>
3334
#include <list>
@@ -84,7 +85,7 @@ class Server : public xpl::iface::Server {
8485
std::shared_ptr<xpl::iface::Timeout_callback> timeout_callback);
8586

8687
std::shared_ptr<xpl::iface::Ssl_context> ssl_context() const override {
87-
return m_ssl_context;
88+
return std::atomic_load(&m_ssl_context);
8889
}
8990

9091
bool reset() override;

‎plugin/x/src/variables/status_variables.cc‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,11 +197,14 @@ int global_status_variable(THD *, SHOW_VAR *var, char *buff) {
197197

198198
if (!server.container()) return 0;
199199

200-
if (!server->ssl_context()) return 0;
200+
// creating a copy of the context's shared_pointer in order to
201+
// extend its lifetime
202+
auto context = server->ssl_context();
203+
if (!context) return 0;
201204

202-
auto &context = server->ssl_context()->options();
205+
auto &options = context->options();
203206
auto context_method = method; // workaround on VC compiler internal error
204-
ReturnType result = (context.*context_method)();
207+
ReturnType result = (options.*context_method)();
205208

206209
mysqld::xpl_show_var(var).assign(result);
207210
return 0;

0 commit comments

Comments
 (0)