8000 fixed memleaks that occurred during failure testing only · thurt/arangodb@f1ed7cd · GitHub
[go: up one dir, main page]
Skip to content

Commit f1ed7cd

Browse files
jsteemannjsteemann
committed
fixed memleaks that occurred during failure testing only
1 parent a8e822e commit f1ed7cd

File tree

3 files changed

+164
-131
lines changed

3 files changed

+164
-131
lines changed

‎arangod/Aql/IndexBlock.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,7 @@ bool IndexBlock::initIndexes () {
319319
}
320320
if (_currentIndex < _indexes.size()) {
321321
// This check will work as long as _indexes.size() < MAX_SIZE_T
322+
TRI_ASSERT(_iterator == nullptr);
322323
_iterator = createIterator();
323324
}
324325
else {
@@ -364,6 +365,7 @@ void IndexBlock::startNextIterator () {
364365
}
365366
if (_currentIndex < _indexes.size()) {
366367
// This check will work as long as _indexes.size() < MAX_SIZE_T
368+
TRI_ASSERT(_iterator == nullptr);
367369
_iterator = createIterator();
368370
}
369371
}

‎arangod/Indexes/HashIndex.cpp

Lines changed: 60 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -904,65 +904,76 @@ IndexIterator* HashIndex::iteratorForCondition (IndexIteratorContext* context,
904904
std::vector<TRI_hash_index_search_value_t*> searchValues;
905905
searchValues.reserve(maxPermutations);
906906

907-
// create all permutations
908-
auto shaper = _collection->getShaper();
909-
size_t current = 0;
910-
bool done = false;
911-
while (! done) {
912-
auto searchValue = std::make_unique<TRI_hash_index_search_value_t>();
913-
searchValue->reserve(n);
914-
915-
bool valid = true;
916-
for (size_t i = 0; i < n; ++i) {
917-
auto& state = permutationStates[i];
918-
std::unique_ptr<TRI_json_t> json(state.getValue()->toJsonValue(TRI_UNKNOWN_MEM_ZONE));
919-
920-
if (json == nullptr) {
921-
valid = false;
922-
break;
923-
}
907+
try {
908+
// create all permutations
909+
auto shaper = _collection->getShaper();
910+
size_t current = 0;
911+
bool done = false;
912+
while (! done) {
913+
auto searchValue = std::make_unique<TRI_hash_index_search_value_t>();
914+
searchValue->reserve(n);
915+
916+
bool valid = true;
917+
for (size_t i = 0; i < n; ++i) {
918+
auto& state = permutationStates[i];
919+
std::unique_ptr<TRI_json_t> json(state.getValue()->toJsonValue(TRI_UNKNOWN_MEM_ZONE));
920+
921+
if (json == nullptr) {
922+
valid = false;
923+
break;
924+
}
924925

925-
auto shaped = TRI_ShapedJsonJson(shaper, json.get(), false);
926-
927-
if (shaped == nullptr) {
928-
// no such shape exists. this means we won't find this value and can go on with the next permutation
929-
valid = false;
930-
break;
926+
auto shaped = TRI_ShapedJsonJson(shaper, json.get(), false);
927+
928+
if (shaped == nullptr) {
929+
// no such shape exists. this means we won't find this value and can go on with the next permutation
930+
valid = false;
931+
break;
932+
}
933+
934+
searchValue->_values[state.attributePosition] = *shaped;
935+
TRI_Free(shaper->memoryZone(), shaped);
931936
}
932-
933-
searchValue->_values[state.attributePosition] = *shaped;
934-
TRI_Free(shaper->memoryZone(), shaped);
935-
}
936937

937-
if (valid) {
938-
searchValues.push_back(searchValue.get());
939-
searchValue.release();
940-
}
941-
942-
// now permute
943-
while (true) {
944-
if (++permutationStates[current].current < permutationStates[current].n) {
945-
current = 0;
946-
// abort inner iteration
947-
break;
938+
if (valid) {
939+
searchValues.push_back(searchValue.get());
940+
searchValue.release();
948941
}
949942

950-
permutationStates[current].current = 0;
943+
// now permute
944+
while (true) {
945+
if (++permutationStates[current].current < permutationStates[current].n) {
946+
current = 0;
947+
// abort inner iteration
948+
break;
949+
}
951950

952-
if (++current >= n) {
953-
done = true;
954-
break;
951+
permutationStates[current].current = 0;
952+
953+
if (++current >= n) {
954+
done = true;
955+
break;
956+
}
957+
// next inner iteration
955958
}
956-
// next inner iteration
957959
}
958-
}
959960

960-
TRI_ASSERT(searchValues.size() <= maxPermutations);
961-
962-
// Create the iterator
963-
TRI_IF_FAILURE("HashIndex::noIterator") {
964-
THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);
961+
TRI_ASSERT(searchValues.size() <= maxPermutations);
962+
963+
// Create the iterator
964+
TRI_IF_FAILURE("HashIndex::noIterator") {
965+
THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);
966+
}
967+
965968
}
969+
catch (...) {
970+
// prevent a leak here
971+
for (auto& it : searchValues) {
972+
delete it;
973+
}
974+
throw;
975+
}
976+
966977
return new HashIndexIterator(this, searchValues);
967978
}
968979

‎arangod/Indexes/SkiplistIndex.cpp

Lines changed: 102 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ static TRI_index_operator_t* buildRangeOperator (TRI_json_t const* lowerBound,
154154
lowerOperator.release();
155155
upperOperator.release();
156156
return rangeOperator.release();
157-
};
157+
}
158158

159159
////////////////////////////////////////////////////////////////////////////////
160160
/// @brief frees an element in the skiplist
@@ -1331,12 +1331,17 @@ IndexIterator* SkiplistIndex::iteratorForCondition (IndexIteratorContext* contex
13311331
nullArray.add(Json(Json::Null));
13321332
std::unique_ptr<TRI_index_operator_t> unboundOperator(TRI_CreateIndexOperator(TRI_GE_INDEX_OPERATOR, nullptr,
13331333
nullptr, nullArray.steal(), _shaper, 1));
1334-
std::vector<TRI_index_operator_t*> searchValues({unboundOperator.get()});
1334+
std::vector<TRI_index_operator_t*> searchValues({ unboundOperator.get() });
13351335
unboundOperator.release();
13361336

1337-
TRI_IF_FAILURE("SkiplistIndex::noSortIterator") {
1337+
TRI_IF_FAILURE("SkiplistIndex::noSortIterator") {
1338+
// prevent a (false-positive) memleak here
1339+
for (auto& it : searchValues) {
1340+
delete it;
1341+
}
13381342
THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);
13391343
}
1344+
13401345
return new SkiplistIndexIterator(this, searchValues, reverse);
13411346
}
13421347

@@ -1477,102 +1482,117 @@ IndexIterator* SkiplistIndex::iteratorForCondition (IndexIteratorContext* contex
14771482
std::vector<TRI_index_operator_t*> searchValues;
14781483
searchValues.reserve(maxPermutations);
14791484

1480-
if (usedFields == 0) {
1481-
// We have a range query based on the first _field
1482-
auto op = buildRangeOperator(lower.get(), includeLower, upper.get(), includeUpper, nullptr, _shaper);
1483-
if (op != nullptr) {
1484-
searchValues.emplace_back(op);
1485-
TRI_IF_FAILURE("SkiplistIndex::onlyRangeOperator") {
1486-
THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);
1487-
}
1488-
}
1489-
}
1490-
else {
1491-
bool done = false;
1492-
// create all permutations
1493-
while (! done) {
1494-
std::unique_ptr<TRI_json_t> parameter(TRI_CreateArrayJson(TRI_UNKNOWN_MEM_ZONE, usedFields));
1495-
1496-
bool valid = true;
1497-
for (size_t i = 0; i < usedFields; ++i) {
1498-
TRI_ASSERT(i < permutationStates.size());
1499-
auto& state = permutationStates[i];
1500-
std::unique_ptr<TRI_json_t> json(state.getValue()->toJsonValue(TRI_UNKNOWN_MEM_ZONE));
1501-
1502-
if (json == nullptr) {
1503-
valid = false;
1504-
break;
1485+
try {
1486+
if (usedFields == 0) {
1487+
// We have a range query based on the first _field
1488+
std::unique_ptr<TRI_index_operator_t> op(buildRangeOperator(lower.get(), includeLower, upper.get(), includeUpper, nullptr, _shaper));
1489+
1490+
if (op != nullptr) {
1491+
searchValues.emplace_back(op.get());
1492+
op.release();
1493+
1494+
TRI_IF_FAILURE("SkiplistIndex::onlyRangeOperator") {
1495+
THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);
15051496
}
1506-
TRI_PushBack3ArrayJson(TRI_UNKNOWN_MEM_ZONE, parameter.get(), json.release());
15071497
}
1508-
1509-
if (valid) {
1510-
std::unique_ptr<TRI_index_operator_t> tmpOp(TRI_CreateIndexOperator(TRI_EQ_INDEX_OPERATOR,
1511-
nullptr,
1512-
nullptr,
1513-
parameter.get(),
1514-
_shaper,
1515-
usedFields));
1516-
// Note we create a new RangeOperator always.
1517-
std::unique_ptr<TRI_index_operator_t> rangeOperator(buildRangeOperator(lower.get(), includeLower, upper.get(), includeUpper, parameter.get(), _shaper));
1518-
parameter.release();
1519-
1520-
if (rangeOperator != nullptr) {
1521-
std::unique_ptr<TRI_index_operator_t> combinedOp(TRI_CreateIndexOperator(TRI_AND_INDEX_OPERATOR,
1522-
tmpOp.get(),
1523-
rangeOperator.get(),
1524-
nullptr,
1525-
_shaper,
1526-
2));
1527-
rangeOperator.release();
1528-
tmpOp.release();
1529-
searchValues.emplace_back(combinedOp.get());
1530-
TRI_IF_FAILURE("SkiplistIndex::rangeOperatorNoTmp") {
1531-
THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);
1498+
}
1499+
else {
1500+
bool done = false;
1501+
// create all permutations
1502+
while (! done) {
1503+
std::unique_ptr<TRI_json_t> parameter(TRI_CreateArrayJson(TRI_UNKNOWN_MEM_ZONE, usedFields));
1504+
1505+
bool valid = true;
1506+
for (size_t i = 0; i < usedFields; ++i) {
1507+
TRI_ASSERT(i < permutationStates.size());
1508+
auto& state = permutationStates[i];
1509+
std::unique_ptr<TRI_json_t> json(state.getValue()->toJsonValue(TRI_UNKNOWN_MEM_ZONE));
1510+
1511+
if (json == nullptr) {
1512+
valid = false;
1513+
break;
15321514
}
1533-
combinedOp.release();
1515+
TRI_PushBack3ArrayJson(TRI_UNKNOWN_MEM_ZONE, parameter.get(), json.release());
15341516
}
1535-
else {
1536-
if (tmpOp != nullptr) {
1537-
searchValues.emplace_back(tmpOp.get());
1538-
TRI_IF_FAILURE("SkiplistIndex::rangeOperatorTmp") {
1517+
1518+
if (valid) {
1519+
std::unique_ptr<TRI_index_operator_t> tmpOp(TRI_CreateIndexOperator(TRI_EQ_INDEX_OPERATOR,
1520+
nullptr,
1521+
nullptr,
1522+
parameter.get(),
1523+
_shaper,
1524+
usedFields));
1525+
// Note we create a new RangeOperator always.
1526+
std::unique_ptr<TRI_index_operator_t> rangeOperator(buildRangeOperator(lower.get(), includeLower, upper.get(), includeUpper, parameter.get(), _shaper));
1527+
parameter.release();
1528+
1529+
if (rangeOperator != nullptr) {
1530+
std::unique_ptr<TRI_index_operator_t> combinedOp(TRI_CreateIndexOperator(TRI_AND_INDEX_OPERATOR,
1531+
tmpOp.get(),
1532+
rangeOperator.get(),
1533+
nullptr,
1534+
_shaper,
1535+
2));
1536+
rangeOperator.release();
1537+
tmpOp.release();
1538+
searchValues.emplace_back(combinedOp.get());
1539+
combinedOp.release();
1540+
TRI_IF_FAILURE("SkiplistIndex::rangeOperatorNoTmp") {
15391541
THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);
15401542
}
1541-
tmpOp.release();
1543+
}
1544+
else {
1545+
if (tmpOp != nullptr) {
1546+
searchValues.emplace_back(tmpOp.get());
1547+
tmpOp.release();
1548+
TRI_IF_FAILURE("SkiplistIndex::rangeOperatorTmp") {
1549+
THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);
1550+
}
1551+
}
15421552
}
15431553
}
1544-
}
15451554

1546-
size_t const np = permutationStates.size() - 1;
1547-
size_t current = 0;
1548-
// now permute
1549-
while (true) {
1550-
if (++permutationStates[np - current].current < permutationStates[np - current].n) {
1551-
current = 0;
1552-
// abort inner iteration
1553-
break;
1554-
}
1555+
size_t const np = permutationStates.size() - 1;
1556+
size_t current = 0;
1557+
// now permute
1558+
while (true) {
1559+
if (++permutationStates[np - current].current < permutationStates[np - current].n) {
1560+
current = 0;
1561+
// abort inner iteration
1562+
break;
1563+
}
15551564

1556-
permutationStates[np - current].current = 0;
1565+
permutationStates[np - current].current = 0;
15571566

1558-
if (++current >= usedFields) {
1559-
done = true;
1560-
break;
1567+
if (++current >= usedFields) {
1568+
done = true;
1569+
break;
1570+
}
1571+
// next inner iteration
15611572
}
1562-
// next inner iteration
15631573
}
15641574
}
1575+
1576+
if (searchValues.empty()) {
1577+
return nullptr;
1578+
}
1579+
1580+
if (reverse) {
1581+
std::reverse(searchValues.begin(), searchValues.end());
1582+
}
1583+
1584+
TRI_IF_FAILURE("SkiplistIndex::noIterator") {
1585+
THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);
1586+
}
15651587
}
1566-
if (searchValues.empty()) {
1567-
return nullptr;
1568-
}
1569-
if (reverse) {
1570-
std::reverse(searchValues.begin(), searchValues.end());
1588+
catch (...) {
1589+
// prevent memleak here
1590+
for (auto& it : searchValues) {
1591+
delete it;
1592+
}
1593+
throw;
15711594
}
15721595

1573-
TRI_IF_FAILURE("SkiplistIndex::noIterator") {
1574-
THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);
1575-
}
15761596
return new SkiplistIndexIterator(this, searchValues, reverse);
15771597
}
15781598

0 commit comments

Comments
 (0)
0