Skip to content

Commit 9ff0be8

Browse files
blauddenblaudden
committed
Bug#33855045 Test ndbinfo_upgrade fails on valgrind builds [#6]
The NdbDictionary::Event returned from NdbDictionar::Dictionary::getEvent() need to be released. Fix by: - Improve function description of NdbDictionary:Dictionary::getEvent() to indicate that the returned Event need to be released. - Add new NdbDictionary:Dictionary::releaseEvent() for releasing the event returned. - Add RAII support to NdbApi for managing the lifetime NdbDictionary::Event using std::unique_ptr, this is enabled when compiling with at least C++11 support. This patch fixes potential memory leaks in various places as well as the one in NdbInfoScanVirtual that was found by valgrind. Change-Id: Ib52133abff1cd28eef17aeb6cb0527de9fab9574
1 parent 49b4621 commit 9ff0be8

14 files changed

+72
-64
lines changed

‎storage/ndb/include/ndbapi/NdbDictionary.hpp

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,13 @@
2525
#ifndef NdbDictionary_H
2626
#define NdbDictionary_H
2727

28+
#if __cplusplus >= 201103L || (defined(_MSVC_LANG) && _MSVC_LANG >= 201103L)
29+
#include <memory>
30+
#endif
31+
2832
#include <ndb_types.h>
2933

30-
class Ndb;
34+
class Ndb;
3135
struct CHARSET_INFO;
3236

3337
/* Forward declaration only. */
@@ -2484,12 +2488,20 @@ class NdbDictionary {
24842488
int dropEvent(const char * eventName, int force= 0);
24852489

24862490
/**
2487-
* Get event with given name.
2491+
* Get event instance for given name.
24882492
* @param eventName Name of event to get.
24892493
* @return an Event if successful, otherwise NULL.
2494+
*
2495+
* @note The returned Event need to be released with `releaseEvent`
24902496
*/
24912497
const Event * getEvent(const char * eventName);
24922498

2499+
/**
2500+
* Release event previously returned from getEvent()
2501+
* @param event The event to release
2502+
*/
2503+
static void releaseEvent(const Event* event);
2504+
24932505
/**
24942506
* List defined events
24952507
* @param list Empty list to hold events returned in the dictionary
@@ -2973,6 +2985,15 @@ class NdbDictionary {
29732985
const void* val);
29742986
static
29752987
class NdbOut& printColumnTypeDescription(class NdbOut &, const Column &);
2988+
2989+
#if __cplusplus >= 201103L || (defined(_MSVC_LANG) && _MSVC_LANG >= 201103L)
2990+
// RAII support for the Event returned from Dictionary::getEvent()
2991+
struct Event_deleter {
2992+
void operator()(const Event *event) { Dictionary::releaseEvent(event); }
2993+
};
2994+
using Event_ptr = std::unique_ptr<const Event, Event_deleter>;
2995+
#endif
2996+
29762997

29772998
}; // class NdbDictionary
29782999

‎storage/ndb/include/ndbapi/NdbEventOperation.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -337,13 +337,13 @@ class NdbEventOperation {
337337
friend class NdbEventOperationImpl;
338338
friend class NdbEventBuffer;
339339
#endif
340-
NdbEventOperation(Ndb *ndb, const NdbDictionary::Event *event);
340+
NdbEventOperation(Ndb *ndb, const NdbDictionary::Event* event);
341341
~NdbEventOperation();
342342
class NdbEventOperationImpl &m_impl;
343343
NdbEventOperation(NdbEventOperationImpl& impl);
344344

345-
NdbEventOperation(const NdbEventOperation&); // Not impl.
346-
NdbEventOperation&operator=(const NdbEventOperation&);
345+
NdbEventOperation(const NdbEventOperation&) = delete;
346+
NdbEventOperation&operator=(const NdbEventOperation&) = delete;
347347
};
348348

349349
typedef void (* NdbEventCallback)(NdbEventOperation*, Ndb*, void*);

‎storage/ndb/plugin/ha_ndbcluster_binlog.cc

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5151,13 +5151,15 @@ int Ndb_binlog_client::create_event(Ndb *ndb,
51515151
}
51525152

51535153
/*
5154-
try retrieving the event, if table version/id matches, we will get
5154+
Try retrieving the event, if table version/id matches, we will get
51555155
a valid event. Otherwise we have an old event from before
51565156
*/
5157-
const NDBEVENT *ev;
5158-
if ((ev = dict->getEvent(event_name.c_str()))) {
5159-
delete ev;
5160-
return 0;
5157+
{
5158+
NdbDictionary::Event_ptr ev(dict->getEvent(event_name.c_str()));
5159+
if (ev) {
5160+
// The event already exists in NDB
5161+
return 0;
5162+
}
51615163
}
51625164

51635165
// Old event from before; an error, but try to correct it

‎storage/ndb/plugin/ndb_binlog_client.cc

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright (c) 2018, 2021, Oracle and/or its affiliates.
2+
Copyright (c) 2018, 2022, Oracle and/or its affiliates.
33
44
This program is free software; you can redistribute it and/or modify
55
it under the terms of the GNU General Public License, version 2.0,
@@ -168,16 +168,12 @@ bool Ndb_binlog_client::event_exists_for_table(Ndb *ndb,
168168
event_name_for_table(m_dbname, m_tabname, share->get_binlog_full());
169169

170170
// Get event from NDB
171-
NdbDictionary::Dictionary *dict = ndb->getDictionary();
172-
const NdbDictionary::Event *existing_event =
173-
dict->getEvent(event_name.c_str());
171+
NdbDictionary::Event_ptr existing_event(
172+
ndb->getDictionary()->getEvent(event_name.c_str()));
174173
if (existing_event) {
175174
// The event exist
176-
delete existing_event;
177-
178175
ndb_log_verbose(1, "Event '%s' for table '%s.%s' already exists",
179176
event_name.c_str(), m_dbname, m_tabname);
180-
181177
return true;
182178
}
183179
return false; // Does not exist

‎storage/ndb/src/ndbapi/NdbBlob.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -245,14 +245,14 @@ NdbBlob::getBlobTable(NdbTableImpl& bt, const NdbTableImpl* t, const NdbColumnIm
245245
int
246246
NdbBlob::getBlobEventName(char* bename, Ndb* anNdb, const char* eventName, const char* columnName)
247247
{
248-
NdbEventImpl* e = anNdb->theDictionary->m_impl.getEvent(eventName);
248+
std::unique_ptr<NdbEventImpl> e(
249+
anNdb->theDictionary->m_impl.getEvent(eventName));
249250
if (e == NULL)
250251
return -1;
251252
NdbColumnImpl* c = e->m_tableImpl->getColumn(columnName);
252253
if (c == NULL)
253254
return -1;
254-
getBlobEventName(bename, e, c);
255-
delete e; // it is from new NdbEventImpl
255+
getBlobEventName(bename, e.get(), c);
256256
return 0;
257257
}
258258

‎storage/ndb/src/ndbapi/NdbDictionary.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3283,6 +3283,11 @@ NdbDictionary::Dictionary::getEvent(const char * eventName)
32833283
return 0;
32843284
}
32853285

3286+
void NdbDictionary::Dictionary::releaseEvent(
3287+
const NdbDictionary::Event *event) {
3288+
delete event;
3289+
}
3290+
32863291
int
32873292
NdbDictionary::Dictionary::listEvents(List& list)
32883293
{

‎storage/ndb/src/ndbapi/NdbDictionaryImpl.cpp

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6287,17 +6287,15 @@ NdbDictionaryImpl::getEvent(const char * eventName, NdbTableImpl* tab)
62876287
DBUG_ENTER("NdbDictionaryImpl::getEvent");
62886288
DBUG_PRINT("enter",("eventName= %s", eventName));
62896289

6290-
NdbEventImpl *ev = new NdbEventImpl();
6290+
std::unique_ptr<NdbEventImpl> ev = std::make_unique<NdbEventImpl>();
62916291
if (ev == NULL) {
62926292
DBUG_RETURN(NULL);
62936293
}
62946294

62956295
ev->setName(eventName);
62966296

6297-
int ret = m_receiver.createEvent(*ev, 1 /* getFlag set */);
6298-
6297+
const int ret = m_receiver.createEvent(*ev, 1 /* getFlag set */);
62996298
if (ret) {
6300-
delete ev;
63016299
DBUG_RETURN(NULL);
63026300
}
63036301

@@ -6309,7 +6307,6 @@ NdbDictionaryImpl::getEvent(const char * eventName, NdbTableImpl* tab)
63096307
if (tab == 0)
63106308
{
63116309
DBUG_PRINT("error",("unable to find table %s", ev->getTableName()));
6312-
delete ev;
63136310
DBUG_RETURN(NULL);
63146311
}
63156312
if ((tab->m_status != NdbDictionary::Object::Retrieved) ||
@@ -6323,7 +6320,6 @@ NdbDictionaryImpl::getEvent(const char * eventName, NdbTableImpl* tab)
63236320
if (tab == 0)
63246321
{
63256322
DBUG_PRINT("error",("unable to find table %s", ev->getTableName()));
6326-
delete ev;
63276323
DBUG_RETURN(NULL);
63286324
}
63296325
}
@@ -6349,15 +6345,13 @@ NdbDictionaryImpl::getEvent(const char * eventName, NdbTableImpl* tab)
63496345
table_version_major(ev->m_table_version))
63506346
{
63516347
m_error.code = 241;
6352-
delete ev;
63536348
DBUG_RETURN(NULL);
63546349
}
63556350

63566351
if ( attributeList_sz > (uint) table.getNoOfColumns() )
63576352
{
63586353
m_error.code = 241;
63596354
DBUG_PRINT("error",("Invalid version, too many columns"));
6360-
delete ev;
63616355
DBUG_RETURN(NULL);
63626356
}
63636357

@@ -6367,7 +6361,6 @@ NdbDictionaryImpl::getEvent(const char * eventName, NdbTableImpl* tab)
63676361
{
63686362
m_error.code = 241;
63696363
DBUG_PRINT("error",("Invalid version, column %d out of range", id));
6370-
delete ev;
63716364
DBUG_RETURN(NULL);
63726365
}
63736366
if (!mask.get(id))
@@ -6419,7 +6412,6 @@ NdbDictionaryImpl::getEvent(const char * eventName, NdbTableImpl* tab)
64196412
{
64206413
DBUG_PRINT("error", ("Failed to get blob event for column %u",
64216414
col->getColumnNo()));
6422-
delete ev;
64236415

64246416
/**
64256417
* DICT will return error code 723 if the event exists but
@@ -6455,11 +6447,11 @@ NdbDictionaryImpl::getEvent(const char * eventName, NdbTableImpl* tab)
64556447
blob_count,
64566448
blob_event_count));
64576449
m_error.code = 241; /* Invalid schema object version */
6458-
delete ev;
64596450
DBUG_RETURN(NULL);
64606451
}
64616452

6462-
DBUG_RETURN(ev);
6453+
// Return the successfully created event
6454+
DBUG_RETURN(ev.release());
64636455
}
64646456

64656457
// ev is main event and has been retrieved previously
@@ -6478,8 +6470,7 @@ NdbDictionaryImpl::getBlobEvent(const NdbEventImpl& ev, uint col_no)
64786470
char bename[MAX_TAB_NAME_SIZE];
64796471
NdbBlob::getBlobEventName(bename, &ev, col);
64806472

6481-
NdbEventImpl* blob_ev = getEvent(bename, blob_tab);
6482-
DBUG_RETURN(blob_ev);
6473+
DBUG_RETURN(getEvent(bename, blob_tab));
64836474
}
64846475

64856476
void
@@ -6649,10 +6640,10 @@ NdbDictionaryImpl::dropEvent(const char * eventName, int force)
66496640
DBUG_ENTER("NdbDictionaryImpl::dropEvent");
66506641
DBUG_PRINT("enter", ("name:%s force: %d", eventName, force));
66516642

6652-
NdbEventImpl *evnt = NULL;
6643+
std::unique_ptr<NdbEventImpl> evnt;
66536644
if (!force)
66546645
{
6655-
evnt = getEvent(eventName); // allocated
6646+
evnt.reset(getEvent(eventName));
66566647
if (evnt == NULL)
66576648
{
66586649
if (m_error.code != 723 && // no such table
@@ -6666,12 +6657,10 @@ NdbDictionaryImpl::dropEvent(const char * eventName, int force)
66666657
}
66676658
if (evnt == NULL)
66686659
{
6669-
evnt = new NdbEventImpl();
6660+
evnt = std::make_unique<NdbEventImpl>();
66706661
evnt->setName(eventName);
66716662
}
6672-
int ret = dropEvent(*evnt);
6673-
delete evnt;
6674-
DBUG_RETURN(ret);
6663+
DBUG_RETURN(dropEvent(*evnt));
66756664
}
66766665

66776666
int

‎storage/ndb/src/ndbapi/NdbEventOperation.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,13 @@
3131

3232
NdbEventOperation::NdbEventOperation(Ndb *ndb,
3333
const NdbDictionary::Event *event)
34-
: m_impl(* new NdbEventOperationImpl(*this, ndb, event))
34+
: m_impl(* new NdbEventOperationImpl(*this, ndb, std::move(event)))
3535
{
3636
}
3737

38+
NdbEventOperation::NdbEventOperation(NdbEventOperationImpl &impl)
39+
: m_impl(impl) {}
40+
3841
NdbEventOperation::~NdbEventOperation()
3942
{
4043
NdbEventOperationImpl * tmp = &m_impl;
@@ -268,9 +271,6 @@ int NdbEventOperation::getNdbdNodeId() const
268271
* Private members
269272
*/
270273

271-
NdbEventOperation::NdbEventOperation(NdbEventOperationImpl& impl)
272-
: m_impl(impl) {}
273-
274274
const struct NdbError &
275275
NdbEventOperation::getNdbError() const {
276276
return m_impl.getNdbError();

‎storage/ndb/src/ndbapi/NdbEventOperationImpl.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4267,14 +4267,14 @@ NdbEventBuffer::createEventOperation(const char* eventName,
42674267
}
42684268

42694269
NdbDictionary::Dictionary *dict = m_ndb->getDictionary();
4270-
const NdbDictionary::Event *event = dict->getEvent(eventName);
4270+
NdbDictionary::Event_ptr event(dict->getEvent(eventName));
42714271
if (!event)
42724272
{
42734273
theError.code= dict->getNdbError().code;
42744274
return nullptr;
42754275
}
42764276

4277-
NdbEventOperation* tOp= new NdbEventOperation(m_ndb, event);
4277+
NdbEventOperation* tOp= new NdbEventOperation(m_ndb, event.release());
42784278
if (tOp == 0)
42794279
{
42804280
theError.code= 4000;

‎storage/ndb/src/ndbapi/NdbEventOperationImpl.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,7 @@ class NdbEventOperationImpl : public NdbEventOperation {
556556
public:
557557
NdbEventOperationImpl(NdbEventOperation &f,
558558
Ndb *ndb,
559-
const NdbDictionary::Event *myEvnt);
559+
const NdbDictionary::Event* event);
560560
NdbEventOperationImpl(Ndb *theNdb,
561561
NdbEventImpl *evnt);
562562
void init();

‎storage/ndb/src/ndbapi/NdbIndexStatImpl.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2911,19 +2911,17 @@ int
29112911
NdbIndexStatImpl::check_sysevents(Ndb* ndb)
29122912
{
29132913
Sys sys(this, ndb);
2914-
NdbDictionary::Dictionary* const dic = ndb->getDictionary();
29152914

29162915
if (check_systables(sys) == -1)
29172916
return -1;
29182917

2919-
const char* const evname = NDB_INDEX_STAT_HEAD_EVENT;
2920-
const NdbDictionary::Event* ev = dic->getEvent(evname);
2921-
if (ev == 0)
2918+
NdbDictionary::Event_ptr ev(
2919+
ndb->getDictionary()->getEvent(NDB_INDEX_STAT_HEAD_EVENT));
2920+
if (ev == nullptr)
29222921
{
2923-
setError(dic->getNdbError().code, __LINE__);
2922+
setError(ndb->getDictionary()->getNdbError().code, __LINE__);
29242923
return -1;
29252924
}
2926-
delete ev; // getEvent() creates new instance
29272925
return 0;
29282926
}
29292927

‎storage/ndb/src/ndbapi/NdbInfoScanVirtual.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1596,7 +1596,7 @@ class EventsTable : public VirtualTable {
15961596
const override {
15971597

15981598
const DictionaryList::Element * elem;
1599-
std::unique_ptr<const NdbDictionary::Event> event;
1599+
NdbDictionary::Event_ptr event;
16001600

16011601
do {
16021602
elem = ctx->nextInList(row);

‎storage/ndb/test/ndbapi/test_event.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright (c) 2003, 2021, Oracle and/or its affiliates.
2+
Copyright (c) 2003, 2022, Oracle and/or its affiliates.
33
44
This program is free software; you can redistribute it and/or modify
55
it under the terms of the GNU General Public License, version 2.0,
@@ -4041,7 +4041,7 @@ int runTryGetEvent(NDBT_Context* ctx, NDBT_Step* step)
40414041
{
40424042
g_err << "Attempting to get the event, expect "
40434043
<< ((odd?"success":"failure")) << endl;
4044-
const NdbDictionary::Event* ev = myDict->getEvent(eventName);
4044+
NdbDictionary::Event_ptr ev(myDict->getEvent(eventName));
40454045

40464046
if (odd)
40474047
{

0 commit comments

Comments
 (0)