Skip to content

Commit

Permalink
Bug#32751506 TEST_MGM MEM LEAKS
Browse files Browse the repository at this point in the history
Memory leaks detected when running testMgm with ASAN build.

bld_asan$> mtr test_mgm

Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x3004ed in malloc
(trunk/bld_asan/runtime_output_directory/testMgm+0x3004ed)
    #1 0x7f794d6b0b46 in ndb_mgm_create_logevent_handle
trunk/bld_asan/../storage/ndb/src/mgmapi/ndb_logevent.cpp:85:24
    #2 0x335b4b in runTestMgmApiReadErrorRestart(NDBT_Context*,
NDBT_Step*)
trunk/bld_asan/../storage/ndb/test/ndbapi/testMgm.cpp:652:32

Add support for using unique_ptr for all functions in mgmapi that return
pointer to something that need to be released.
Move existing functionality for ndb_mgm_configuration to same new file.
Use ndb_mgm namespace for new functions and remove implementation
details from both the new and old functionality
Use new functionality to properly release allocated memory.

Change-Id: Id455234077c4ed6756e93bf7f40a1e93179af1a0
  • Loading branch information
blaudden committed Apr 22, 2021
1 parent a322f26 commit 6d43588
Show file tree
Hide file tree
Showing 15 changed files with 141 additions and 64 deletions.
26 changes: 0 additions & 26 deletions storage/ndb/include/mgmapi/mgmapi.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,6 @@
#ifndef MGMAPI_H
#define MGMAPI_H

#if defined(__cplusplus)
#if __cplusplus >= 201103L || (defined(_MSVC_LANG) && _MSVC_LANG >= 201103L)
#include <memory> // std::unique_ptr for ndb_mgm_config_unique_ptr
#endif
#endif

#ifdef _WIN32
#include <WinSock2.h>
#include <ws2tcpip.h>
Expand Down Expand Up @@ -1583,26 +1577,6 @@ extern "C" {
}
#endif

#if defined(__cplusplus)
#if __cplusplus >= 201103L || (defined(_MSVC_LANG) && _MSVC_LANG >= 201103L)
/*
* Helper class to ease use of C++11 unique pointer with
* ndb_mgm_configuration.
*/
struct ndb_mgm_configuration_deleter
{
void operator()(ndb_mgm_configuration* conf)
{
ndb_mgm_destroy_configuration(conf);
}
};

using ndb_mgm_config_unique_ptr =
std::unique_ptr<ndb_mgm_configuration, ndb_mgm_configuration_deleter>;

#endif
#endif

/** @} */

#endif
Expand Down
7 changes: 4 additions & 3 deletions storage/ndb/include/mgmcommon/ConfigRetriever.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

#include <ndb_types.h>
#include <mgmapi.h>
#include "mgmcommon/NdbMgm.hpp"
#include <BaseString.hpp>

/**
Expand Down Expand Up @@ -57,7 +58,7 @@ class ConfigRetriever {
*
* @return ndb_mgm_config_unique_ptr which may be empty on failure
*/
ndb_mgm_config_unique_ptr getConfig(Uint32 nodeid);
ndb_mgm::config_ptr getConfig(Uint32 nodeid);

void resetError();
int hasError();
Expand All @@ -75,12 +76,12 @@ class ConfigRetriever {
/**
* Get config using socket
*/
ndb_mgm_config_unique_ptr getConfig(NdbMgmHandle handle);
ndb_mgm::config_ptr getConfig(NdbMgmHandle handle);

/**
* Get config from file
*/
ndb_mgm_config_unique_ptr getConfig(const char * file);
ndb_mgm::config_ptr getConfig(const char * file);

/**
* Verify config
Expand Down
93 changes: 93 additions & 0 deletions storage/ndb/include/mgmcommon/NdbMgm.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/*
Copyright (c) 2021, Oracle and/or its affiliates.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License, version 2.0,
as published by the Free Software Foundation.
This program is also distributed with certain software (including
but not limited to OpenSSL) that is licensed under separate terms,
as designated in a particular file or component or in included license
documentation. The authors of MySQL hereby grant you an additional
permission to link the program and your derivative works with the
separately licensed software that they have included with MySQL.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License, version 2.0, for more details.
You should have received a copy of the GNU General Public License
along with this program; if not, write to the Free Software
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/

#ifndef NDB_NDBMGM_HPP
#define NDB_NDBMGM_HPP

#include <memory>

#include "mgmapi/mgmapi.h"

namespace ndb_mgm {

/*
Helper classes to own, manage and dispose objects created using the
MySQL Cluster Management API (MGM API)
*/

// ndb_mgm_handle
struct handle_deleter {
void operator()(ndb_mgm_handle *handle) { ndb_mgm_destroy_handle(&handle); }
};
using handle_ptr = std::unique_ptr<ndb_mgm_handle, handle_deleter>;

// ndb_mgm_configuration
struct config_deleter {
void operator()(ndb_mgm_configuration *conf) {
ndb_mgm_destroy_configuration(conf);
}
};
using config_ptr = std::unique_ptr<ndb_mgm_configuration, config_deleter>;

// ndb_mgm_configuration_iterator
struct config_iter_deleter {
void operator()(ndb_mgm_configuration_iterator *iter) {
ndb_mgm_destroy_iterator(iter);
}
};
using config_iter_ptr =
std::unique_ptr<ndb_mgm_configuration_iterator, config_iter_deleter>;

// ndb_mgm_cluster_state
struct cluster_state_deleter {
void operator()(ndb_mgm_cluster_state *state) { free(state); }
};
using cluster_state_ptr =
std::unique_ptr<ndb_mgm_cluster_state, cluster_state_deleter>;

// ndb_mgm_cluster2_state
struct cluster_state2_deleter {
void operator()(ndb_mgm_cluster_state2 *state) { free(state); }
};
using cluster_state2_ptr =
std::unique_ptr<ndb_mgm_cluster_state2, cluster_state2_deleter>;

// ndb_log_event_handle
struct logevent_handle_deleter {
void operator()(ndb_logevent_handle *handle) {
ndb_mgm_destroy_logevent_handle(&handle);
}
};
using logevent_handle_ptr =
std::unique_ptr<ndb_logevent_handle, logevent_handle_deleter>;

// ndb_mgm_events
struct events_deleter {
void operator()(ndb_mgm_events *events) { free(events); }
};
using events_ptr = std::unique_ptr<ndb_mgm_events, events_deleter>;

} // namespace ndb_mgm

#endif
12 changes: 6 additions & 6 deletions storage/ndb/src/common/mgmcommon/ConfigRetriever.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ ConfigRetriever::is_connected(void)
return (ndb_mgm_is_connected(m_handle) != 0);
}

ndb_mgm_config_unique_ptr
ndb_mgm::config_ptr
ConfigRetriever::getConfig(Uint32 nodeid)
{
if (!m_handle)
Expand All @@ -166,7 +166,7 @@ ConfigRetriever::getConfig(Uint32 nodeid)
const Uint32 save_nodeid = get_configuration_nodeid();
setNodeId(nodeid);

ndb_mgm_config_unique_ptr conf = getConfig(m_handle);
ndb_mgm::config_ptr conf = getConfig(m_handle);

setNodeId(save_nodeid);

Expand All @@ -179,11 +179,11 @@ ConfigRetriever::getConfig(Uint32 nodeid)
return conf;
}

ndb_mgm_config_unique_ptr
ndb_mgm::config_ptr
ConfigRetriever::getConfig(NdbMgmHandle mgm_handle)
{
const int from_node = 0;
ndb_mgm_config_unique_ptr conf(
ndb_mgm::config_ptr conf(
ndb_mgm_get_configuration2(mgm_handle,
m_version,
m_node_type,
Expand All @@ -198,7 +198,7 @@ ConfigRetriever::getConfig(NdbMgmHandle mgm_handle)
return conf;
}

ndb_mgm_config_unique_ptr
ndb_mgm::config_ptr
ConfigRetriever::getConfig(const char * filename)
{
if (access(filename, F_OK))
Expand Down Expand Up @@ -236,7 +236,7 @@ ConfigRetriever::getConfig(const char * filename)
setError(CR_ERROR, "Error while unpacking");
return {};
}
return ndb_mgm_config_unique_ptr(
return ndb_mgm::config_ptr(
reinterpret_cast<ndb_mgm_configuration *>(cvf.getConfigValues()));
}

Expand Down
2 changes: 1 addition & 1 deletion storage/ndb/src/kernel/angel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ angel_run(const char* progname,
}
g_eventLogger->info("Angel allocated nodeid: %u", nodeid);

ndb_mgm_config_unique_ptr config(retriever.getConfig(nodeid));
const ndb_mgm::config_ptr config(retriever.getConfig(nodeid));
if (!config)
{
g_eventLogger->error("Could not fetch configuration/invalid "
Expand Down
3 changes: 2 additions & 1 deletion storage/ndb/src/kernel/vm/Configuration.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

#include <util/BaseString.hpp>
#include <mgmapi.h>
#include "mgmcommon/NdbMgm.hpp"
#include <kernel_types.h>
#include <NdbMutex.h>
#include <NdbThread.h>
Expand Down Expand Up @@ -183,7 +184,7 @@ class Configuration {

ndb_mgm_configuration * m_ownConfig;
const class ConfigValues* get_own_config_values();
ndb_mgm_config_unique_ptr m_clusterConfig;
ndb_mgm::config_ptr m_clusterConfig;
UtilBuffer m_clusterConfigPacked_v1;
UtilBuffer m_clusterConfigPacked_v2;

Expand Down
3 changes: 2 additions & 1 deletion storage/ndb/src/mgmclient/CommandInterpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <ndb_global.h>

#include <mgmapi.h>
#include "mgmcommon/NdbMgm.hpp"
#include <ndbd_exit_codes.h>

#include <util/BaseString.hpp>
Expand Down Expand Up @@ -1969,7 +1970,7 @@ CommandInterpreter::executeShow(char* parameters)
}
NdbAutoPtr<char> ap1((char*)state);

ndb_mgm_config_unique_ptr conf(ndb_mgm_get_configuration(m_mgmsrv, 0));
const ndb_mgm::config_ptr conf(ndb_mgm_get_configuration(m_mgmsrv, 0));
if (!conf) {
ndbout_c("Could not get configuration");
printError();
Expand Down
4 changes: 2 additions & 2 deletions storage/ndb/src/mgmsrv/ConfigManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2182,7 +2182,7 @@ ConfigManager::fetch_config(void)
}
}
// read config from other management server
ndb_mgm_config_unique_ptr conf =
ndb_mgm::config_ptr conf =
m_config_retriever.getConfig(m_config_retriever.get_mgmHandle());

// Disconnect from other mgmd
Expand Down Expand Up @@ -2350,7 +2350,7 @@ ConfigManager::failed_config_change_exists() const
Config*
ConfigManager::load_saved_config(const BaseString& config_name)
{
ndb_mgm_config_unique_ptr retrieved_config =
ndb_mgm::config_ptr retrieved_config =
m_config_retriever.getConfig(config_name.c_str());
if(!retrieved_config)
{
Expand Down
2 changes: 1 addition & 1 deletion storage/ndb/src/ndbapi/ndb_cluster_connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1443,7 +1443,7 @@ int Ndb_cluster_connection_impl::connect(int no_retries,
break;
}

ndb_mgm_config_unique_ptr config = m_config_retriever->getConfig(nodeId);
const ndb_mgm::config_ptr config = m_config_retriever->getConfig(nodeId);
if (!config)
break;

Expand Down
3 changes: 2 additions & 1 deletion storage/ndb/test/include/NdbMgmd.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#define NDB_MGMD_HPP

#include <mgmapi.h>
#include "mgmcommon/NdbMgm.hpp"
#include "../../src/mgmapi/mgmapi_internal.h"

#include <BaseString.hpp>
Expand Down Expand Up @@ -372,7 +373,7 @@ class NdbMgmd {
return false;
}

ndb_mgm_config_unique_ptr conf(ndb_mgm_get_configuration(m_handle,0));
ndb_mgm::config_ptr conf(ndb_mgm_get_configuration(m_handle,0));
if (!conf) {
error("get_config: ndb_mgm_get_configuration failed");
return false;
Expand Down
3 changes: 2 additions & 1 deletion storage/ndb/test/include/NdbRestarter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#define NDBT_RESTARTER_HPP

#include <mgmapi.h>
#include "mgmcommon/NdbMgm.hpp"
#include <Vector.hpp>
#include <BaseString.hpp>

Expand Down Expand Up @@ -179,7 +180,7 @@ class NdbRestarter {

bool connected;
BaseString addr;
ndb_mgm_config_unique_ptr m_config;
ndb_mgm::config_ptr m_config;
bool m_reconnect;
protected:
const ndb_mgm_configuration * getConfig();
Expand Down
Loading

0 comments on commit 6d43588

Please sign in to comment.