Fix lsan error on Aggregator

[cpjulia]

Scope & Purpose

This PR fixes the following error on circle ci nightly for devel:

==arangod==97==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 95971 byte(s) in 181 object(s) allocated from:
#0 0x55bcedc08651 in operator new[](unsigned long) (/root/project/build/bin/arangod+0x5ff0651) (BuildId: 56ec745b02c4b8a51dcd1182073f638c41b8d836)
#1 0x55bcef3ce6b2 in arangodb::aql::AqlValue::initFromSlice(arangodb::velocypack::Slice, unsigned long) /root/project/arangod/Aql/AqlValue.cpp:1305
#2 0x55bcef3ccd75 in AqlValue /root/project/arangod/Aql/AqlValue.cpp:1230
#3 0x55bcef3ccd75 in arangodb::aql::AqlValue::clone() const /root/project/arangod/Aql/AqlValue.cpp
#4 0x55bcef412a24 in (anonymous namespace)::AggregatorMin::reduce(arangodb::aql::AqlValue const&) /root/project/arangod/Aql/Aggregator.cpp:189
#5 0x55bcf16006bc in arangodb::aql::SortedCollectExecutor::CollectGroup::addLine(arangodb::aql::InputAqlItemRow const&) /root/project/arangod/Aql/Executor/SortedCollectExecutor.cpp
#6 0x55bcf15fd63a in arangodb::aql::SortedCollectExecutor::produceRows(arangodb::aql::AqlItemBlockInputRange&, arangodb::aql::OutputAqlItemRow&) /root/project/arangod/Aql/Executor/SortedCollectExecutor.cpp
#7 0x55bcf160b33c in executeProduceRows /root/project/arangod/Aql/ExecutionBlockImpl.tpp:1203
#8 0x55bcf160b33c in arangodb::aql::ExecutionBlockImplarangodb::aql::SortedCollectExecutor::executeWithoutTrace(arangodb::aql::AqlCallStack const&) /root/project/arangod/Aql/ExecutionBlockImpl.tpp:2020
#9 0x55bcf1605f32 in arangodb::aql::ExecutionBlockImplarangodb::aql::SortedCollectExecutor::execute(arangodb::aql::AqlCallStack const&)

/root/project/arangod/Aql/ExecutionBlockImpl.tpp:602

When a query uses aggregators AggregatorMin and AggregatorMax, and the next AqlValue that is parsed is reduced, a deep copy of this AqlValue is made and the old one is tossed out. When the query aborted, the newly allocated value wasn't being cleared out, hence, lsan complained about a memory leak.
This PR fixes it by changing the AggregatorMin and AggregatorMax reset() methods from value.erase() to value.destroy(). Just using value.erase() wouldn't properly reset the value, because it just zeroes out the AqlValue stored in the object without properly freeing its memory. The method destroy() does it all for memory managed values, and erase() just for inline values.

Also, for safety, it also changes the memory accounting in reduce() to precharge the ResourceUsageScope before any memory deallocation is made. This avoids a breach in between the two previous lines that decreased and then increased the usage scope, and also, as increase() can throw, the fix avoids it from throwing an exception after the memory deallocation is made in destroy().

• 💩 Bugfix
• 🍕 New feature
• 🔥 Performance improvement
• 🔨 Refactoring/simplification

Checklist

• Tests
  • Regression tests
  • C++ Unit tests
  • integration tests
  • resilience tests
• 📖 CHANGELOG entry made Obs.: NOT SURE IF NEEDED
• 📚 documentation written (release notes, API changes, ...)
• Backports
  • Backport for 3.12.0: (Please link PR)
  • Backport for 3.11: (Please link PR)
  • Backport for 3.10: (Please link PR)

Related Information

(Please reference tickets / specification / other PRs etc)

• Docs PR:
• Enterprise PR:
• GitHub issue / Jira ticket:
• Design document:

[cursor]

You have run out of free Bugbot PR reviews for this billing cycle. This will reset on October 28.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

[mpoeter]

LGTM!

[mchacki]

LGTM