wbtiger
/
foundationdb
mirror of https://github.com/apple/foundationdb.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3407 from sfc-gh-tclinkenbeard/fix-vectorref-memory-leaks 

Fix memory leaks in VectorRef
This commit is contained in:
Steve Atherton 2020-06-25 00:54:33 -07:00 committed by GitHub
parent 5cd3dad986 3f6222a04d
commit af6b141736
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 54 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
fdbclient/ClusterInterface.h
View File

@ -192,6 +192,8 @@ struct SystemFailureStatus {
}
};
TRIVIALLY_DESTRUCTIBLE(SystemFailureStatus); // Allows VectorRef<SystemFailureStatus>
struct FailureMonitoringReply {
constexpr static FileIdentifier file_identifier = 6820325;
VectorRef< SystemFailureStatus > changes;

10
fdbserver/RestoreMaster.actor.cpp
View File

@ -54,7 +54,7 @@ ACTOR static Future<Void> recruitRestoreRoles(Reference<RestoreWorkerData> maste
ACTOR static Future<Void> distributeRestoreSysInfo(Reference<RestoreMasterData> masterData,
KeyRangeMap<Version>* pRangeVersions);
ACTOR static Future<Standalone<VectorRef<RestoreRequest>>> collectRestoreRequests(Database cx);
ACTOR static Future<std::vector<RestoreRequest>> collectRestoreRequests(Database cx);
ACTOR static Future<Void> initializeVersionBatch(std::map<UID, RestoreApplierInterface> appliersInterf,
std::map<UID, RestoreLoaderInterface> loadersInterf, int batchIndex);
ACTOR static Future<Void> notifyApplierToApplyMutations(Reference<MasterBatchData> batchData,
@ -193,7 +193,7 @@ ACTOR Future<Void> distributeRestoreSysInfo(Reference<RestoreMasterData> masterD
// and ask all restore roles to quit.
ACTOR Future<Void> startProcessRestoreRequests(Reference<RestoreMasterData> self, Database cx) {
state UID randomUID = deterministicRandom()->randomUniqueID();
state Standalone<VectorRef<RestoreRequest>> restoreRequests = wait(collectRestoreRequests(cx));
state std::vector<RestoreRequest> restoreRequests = wait(collectRestoreRequests(cx));
state int restoreIndex = 0;
TraceEvent("FastRestoreMasterWaitOnRestoreRequests", self->id()).detail("RestoreRequests", restoreRequests.size());
@ -636,8 +636,8 @@ void splitKeyRangeForAppliers(Reference<MasterBatchData> batchData,
batchData->samples.clear();
}
ACTOR static Future<Standalone<VectorRef<RestoreRequest>>> collectRestoreRequests(Database cx) {
state Standalone<VectorRef<RestoreRequest>> restoreRequests;
ACTOR static Future<std::vector<RestoreRequest>> collectRestoreRequests(Database cx) {
state std::vector<RestoreRequest> restoreRequests;
state Future<Void> watch4RestoreRequest;
state ReadYourWritesTransaction tr(cx);
@ -657,7 +657,7 @@ ACTOR static Future<Standalone<VectorRef<RestoreRequest>>> collectRestoreRequest
ASSERT(!restoreRequestValues.more);
if (restoreRequestValues.size()) {
for (auto& it : restoreRequestValues) {
restoreRequests.push_back(restoreRequests.arena(), decodeRestoreRequestValue(it.value));
restoreRequests.push_back(decodeRestoreRequestValue(it.value));
TraceEvent("FastRestoreMasterPhaseCollectRestoreRequests")
.detail("RestoreRequest", restoreRequests.back().toString());
}

2
fdbserver/VersionedBTree.actor.cpp
View File

@ -2732,6 +2732,8 @@ struct RedwoodRecordRef {
}
};
TRIVIALLY_DESTRUCTIBLE(RedwoodRecordRef); // Allows VectorRef<RedwoodRecordRef>
struct BTreePage {
typedef DeltaTree<RedwoodRecordRef> BinaryTree;
typedef DeltaTree<RedwoodRecordRef, RedwoodRecordRef::DeltaValueOnly> ValueTree;

5
fdbserver/WorkerInterface.actor.h
View File

@ -639,6 +639,11 @@ struct DebugEntryRef {
}
};
// TODO: This is necessary because IPAddress uses boost::variant
// instead of std::variant. If we switch to std::variant, we don't
// need to explicitly mark DebugEntryRef trivially destructible
TRIVIALLY_DESTRUCTIBLE(DebugEntryRef); // Allows VectorRef<DebugEntryRef>
struct DiskStoreRequest {
constexpr static FileIdentifier file_identifier = 1986262;
bool includePartialStores;

14
fdbserver/workloads/ApiCorrectness.actor.cpp
View File

@ -357,9 +357,8 @@ public:
//results were the same
ACTOR Future<bool> runGet(VectorRef<KeyValueRef> data, int numReads, ApiCorrectnessWorkload *self) {
//Generate a set of random keys to get
state Standalone<VectorRef<Key>> keys;
for(int i = 0; i < numReads; i++)
keys.push_back(keys.arena(), self->selectRandomKey(data, 0.9));
state Standalone<VectorRef<KeyRef>> keys;
for (int i = 0; i < numReads; i++) keys.push_back_deep(keys.arena(), self->selectRandomKey(data, 0.9));
state vector<Optional<Value>> values;
@ -554,9 +553,9 @@ public:
//results were the same
ACTOR Future<bool> runGetKey(VectorRef<KeyValueRef> data, int numGetKeys, ApiCorrectnessWorkload *self) {
//Generate a set of random key selectors
state Standalone<VectorRef<KeySelector>> selectors;
state Standalone<VectorRef<KeySelectorRef>> selectors;
for(int i = 0; i < numGetKeys; i++)
selectors.push_back(selectors.arena(), self->generateKeySelector(data, 100));
selectors.push_back_deep(selectors.arena(), self->generateKeySelector(data, 100));
state Standalone<VectorRef<KeyRef>> keys;
@ -640,9 +639,8 @@ public:
ACTOR Future<bool> runClear(VectorRef<KeyValueRef> data, int numClears, ApiCorrectnessWorkload *self)
{
//Generate a random set of keys to clear
state Standalone<VectorRef<Key>> keys;
for(int i = 0; i < numClears; i++)
keys.push_back(keys.arena(), self->selectRandomKey(data, 0.9));
state Standalone<VectorRef<KeyRef>> keys;
for (int i = 0; i < numClears; i++) keys.push_back_deep(keys.arena(), self->selectRandomKey(data, 0.9));
state int currentIndex = 0;
while(currentIndex < keys.size()) {

2
fdbserver/workloads/TPCCWorkload.h
View File

@ -295,4 +295,6 @@ const std::vector<std::string> syllables = {
} // namespace TPCCWorkload
TRIVIALLY_DESTRUCTIBLE(TPCCWorkload::OrderLine); // Allows VectorRef<TPCCWorkload::OrderLine>
#endif

21
flow/Arena.h
View File

@ -313,6 +313,23 @@ private:
bool valid;
};
// This class is necessary because Optional<T> is effectively trivially
// destructible if T is trivially destructible, but we can't SFINAE out
// destructors until C++20, so we still define the destructor for
// Optional<T> even if it's trivial
template <class T>
struct trivially_destructible : public std::is_trivially_destructible<T> {};
template <class T>
struct trivially_destructible<Optional<T>> : public trivially_destructible<T> {};
// If std::is_trivially_destructible_v<T> is false only because T has a field
// Optional<U> where U is trivially destructible, we can manually specify that
// T is trivially_destructible
#define TRIVIALLY_DESTRUCTIBLE(T) \
template <> \
struct trivially_destructible<T> : std::true_type {}
template<class T>
struct Traceable<Optional<T>> : std::conditional<Traceable<T>::value, std::true_type, std::false_type>::type {
static std::string toString(const Optional<T>& value) {
@ -794,7 +811,9 @@ public:
using value_type = T;
static_assert(SerStrategy == VecSerStrategy::FlatBuffers || string_serialized_traits<T>::value);
// T must be trivially destructible (and copyable)!
// T must be trivially copyable!
// T must be trivially destructible, because ~T is never called
static_assert(trivially_destructible<T>::value);
VectorRef() : data(0), m_size(0), m_capacity(0) {}
template <VecSerStrategy S>

6
flow/flat_buffers.cpp
View File

@ -480,15 +480,15 @@ TEST_CASE("/flow/FlatBuffers/VectorRef") {
}
TEST_CASE("/flow/FlatBuffers/Standalone") {
Standalone<VectorRef<StringRef>> vecIn;
std::vector<Standalone<StringRef>> vecIn;
auto numElements = deterministicRandom()->randomInt(1, 20);
for (int i = 0; i < numElements; ++i) {
auto str = deterministicRandom()->randomAlphaNumeric(deterministicRandom()->randomInt(0, 30));
vecIn.push_back(vecIn.arena(), StringRef(vecIn.arena(), str));
vecIn.push_back(Standalone<StringRef>(str));
}
Standalone<StringRef> value = ObjectWriter::toValue(vecIn, Unversioned());
ArenaObjectReader reader(value.arena(), value, Unversioned());
VectorRef<Standalone<StringRef>> vecOut;
std::vector<Standalone<StringRef>> vecOut;
reader.deserialize(vecOut);
ASSERT(vecOut.size() == vecIn.size());
for (int i = 0; i < vecOut.size(); ++i) {