Skip to content

Commit c357395

Browse files
Bug#35594709: Memory leak on xcom_tcp_server_startup()
Problem ================================ Group Replication ASAN run failing without any symptom of a leak, but with shutdown issues: worker[6] Shutdown report from /dev/shm/mtr-3771884/var-gr-debug/6/log/mysqld.1.err after tests: group_replication.gr_flush_logs group_replication.gr_delayed_initialization_thread_handler_error group_replication.gr_sbr_verifications group_replication.gr_server_uuid_matches_group_name_bootstrap group_replication.gr_stop_async_on_stop_gr group_replication.gr_certifier_message_same_member group_replication.gr_ssl_mode_verify_identity_error_xcom Analysis and Fix ================================ It ended up being a leak on gr_ssl_mode_verify_identity_error_xcom test: Direct leak of 24 byte(s) in 1 object(s) allocated from: #0 0x7f1709fbe1c7 in operator new(unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:99 #1 0x7f16ea0df799 in xcom_tcp_server_startup(Xcom_network_provider*) (/export/home/tmp/BUG35594709/mysql-trunk/BIN-ASAN/plugin_output_directory /group_replication.so+0x65d799) #2 0x7f170751e2b2 (/lib/x86_64-linux-gnu/libstdc++.so.6+0xdc2b2) This happens because we delegated incoming connections cleanup to the external consumer in incoming_connection_task. Since it calls incoming_connection() from Network_provider_manager, in case of a concurrent stop, a connection could be left orphan in the shared atomic due to the lack of an Active Provider, thus creating a memory leak. The solution is to make this cleanup on Network_provider_manager, on both stop_provider() and in stop_all_providers() methods, thus ensuring that no incoming connection leaks. Change-Id: I2367c37608ad075dee63785e9f908af5e81374ca
1 parent f3f6b05 commit c357395

File tree

2 files changed

+22
-1
lines changed

2 files changed

+22
-1
lines changed

plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/network/network_provider_manager.cc

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,14 @@ bool Network_provider_manager::start_network_provider(
102102

103103
bool Network_provider_manager::stop_all_network_providers() {
104104
bool retval = false;
105+
105106
for (auto &&i : m_network_providers) {
106107
// Logical Sum of all stop() operations. If any of the operations fail,
107108
// it will report the whole operation as botched, but it will stop all
108109
// providers
109110
retval |= i.second->stop().first;
111+
112+
this->cleanup_incoming_connection(*(i.second));
110113
}
111114

112115
set_incoming_connections_protocol(get_running_protocol());
@@ -118,7 +121,13 @@ bool Network_provider_manager::stop_network_provider(
118121
enum_transport_protocol provider_key) {
119122
auto net_provider = this->get_provider(provider_key);
120123

121-
return net_provider ? net_provider->stop().first : true;
124+
auto cleanup_and_stop = [&]() {
125+
this->cleanup_incoming_connection(*net_provider);
126+
127+
return net_provider->stop().first;
128+
};
129+
130+
return net_provider ? cleanup_and_stop() : true;
122131
}
123132

124133
const std::shared_ptr<Network_provider>
@@ -383,3 +392,13 @@ void Network_provider_manager::finalize_secure_connections_context() {
383392
CLEANUP_NET_PARAMS_FIELD(tls_params.tls_version);
384393
CLEANUP_NET_PARAMS_FIELD(tls_params.tls_ciphersuites);
385394
}
395+
396+
void Network_provider_manager::cleanup_incoming_connection(
397+
Network_provider &provider_ref) {
398+
Network_connection *remaining_connection = provider_ref.get_new_connection();
399+
400+
if (remaining_connection != nullptr) {
401+
provider_ref.close_connection(*remaining_connection);
402+
delete remaining_connection;
403+
}
404+
}

plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/network/network_provider_manager.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,8 @@ class Network_provider_manager : public Network_provider_management_interface,
374374
m_incoming_connections_protocol = value;
375375
}
376376

377+
void cleanup_incoming_connection(Network_provider &provider_ref);
378+
377379
std::unordered_map<enum_transport_protocol, std::shared_ptr<Network_provider>,
378380
std::hash<int>>
379381
m_network_providers;

0 commit comments

Comments
 (0)