Fix memory leak

The map containing global configuration data had keys of type StringRef,
referencing data allocated in history arenas. When the old history
was deleted, this memory was no longer valid and some keys would point
to garbage memory.

fdbclient/GlobalConfig.actor.cpp

@@ -37,7 +37,7 @@ GlobalConfig& GlobalConfig::globalConfig() {
 	return *reinterpret_cast<GlobalConfig*>(res);
 }
 
-const std::any GlobalConfig::get(StringRef name) {
+const std::any GlobalConfig::get(KeyRef name) {
 	auto it = data.find(name);
 	if (it == data.end()) {
 		return std::any{};
@@ -45,21 +45,48 @@ const std::any GlobalConfig::get(StringRef name) {
 	return it->second;
 }
 
+const std::map<KeyRef, std::any> GlobalConfig::get(KeyRangeRef range) {
+	std::map<KeyRef, std::any> results;
+	for (const auto& [key, value] : data) {
+		if (range.contains(key)) {
+			results[key] = value;
+		}
+	}
+	return results;
+}
+
 Future<Void> GlobalConfig::onInitialized() {
 	return initialized.getFuture();
 }
 
 void GlobalConfig::insert(KeyRef key, ValueRef value) {
+	KeyRef stableKey = KeyRef(arena, key);
 	Tuple t = Tuple::unpack(value);
 	if (t.getType(0) == Tuple::ElementType::UTF8) {
-		data[key] = t.getString(0);
+		data[stableKey] = t.getString(0);
 	} else if (t.getType(0) == Tuple::ElementType::INT) {
-		data[key] = t.getInt(0);
+		data[stableKey] = t.getInt(0);
 	} else if (t.getType(0) == Tuple::ElementType::FLOAT) {
-		data[key] = t.getFloat(0);
+		data[stableKey] = t.getFloat(0);
 	} else if (t.getType(0) == Tuple::ElementType::DOUBLE) {
-		data[key] = t.getDouble(0);
+		data[stableKey] = t.getDouble(0);
 	} else {
 		ASSERT(false);
 	}
 }
+
+void GlobalConfig::erase(KeyRef key) {
+	erase(KeyRangeRef(key, keyAfter(key)));
+}
+
+void GlobalConfig::erase(KeyRangeRef range) {
+	// TODO: Memory leak -- memory for key remains allocated in arena
+	auto it = data.begin();
+	while (it != data.end()) {
+		if (range.contains(it->first)) {
+			it = data.erase(it);
+		} else {
+			++it;
+		}
+	}
+}

fdbclient/GlobalConfig.actor.h

@@ -27,6 +27,7 @@
 #define FDBCLIENT_GLOBALCONFIG_ACTOR_H
 
 #include <any>
+#include <map>
 #include <unordered_map>
 
 #include "fdbclient/CommitProxyInterface.h"
@@ -47,11 +48,14 @@ public:
 
 	static void create(DatabaseContext* cx, Reference<AsyncVar<ClientDBInfo>> dbInfo);
 	static GlobalConfig& globalConfig();
-	const std::any get(StringRef name);
+	const std::any get(KeyRef name);
+	const std::map<KeyRef, std::any> get(KeyRangeRef range);
 	Future<Void> onInitialized();
 
 private:
 	void insert(KeyRef key, ValueRef value);
+	void erase(KeyRef key);
+	void erase(KeyRangeRef range);
 
 	ACTOR static Future<Void> refresh(GlobalConfig* self) {
 		Transaction tr(self->cx);
@@ -83,28 +87,19 @@ private:
 					// version. Mutation history should already be stored in
 					// ascending version order.
 					for (int i = 0; i < history.size(); ++i) {
-						std::pair<Version, VectorRef<MutationRef>> pair = history[i].contents();
+						const std::pair<Version, VectorRef<MutationRef>>& pair = history[i].contents();
 
 						Version version = pair.first;
 						if (version <= self->lastUpdate) {
 							continue;  // already applied this mutation
 						}
 
-						VectorRef<MutationRef>& mutations = pair.second;
+						const VectorRef<MutationRef>& mutations = pair.second;
 						for (const auto& mutation : mutations) {
 							if (mutation.type == MutationRef::SetValue) {
 								self->insert(mutation.param1, mutation.param2);
 							} else if (mutation.type == MutationRef::ClearRange) {
-								// TODO: Could be optimized if using std::map..
-								KeyRangeRef range(mutation.param1, mutation.param2);
-								auto it = self->data.begin();
-								while (it != self->data.end()) {
-									if (range.contains(it->first)) {
-										it = self->data.erase(it);
-									} else {
-										++it;
-									}
-								}
+								self->erase(KeyRangeRef(mutation.param1, mutation.param2));
 							} else {
 								ASSERT(false);
 							}
@@ -123,8 +118,7 @@ private:
 	Database cx;
 	Future<Void> _updater;
 	Promise<Void> initialized;
-	// TODO: Arena to store all data in
-	// TODO: Change to std::map for faster range access
+	Arena arena;
 	std::unordered_map<StringRef, std::any> data;
 	Version lastUpdate;
 };

fdbclient/SpecialKeySpace.actor.cpp

@@ -1377,23 +1377,26 @@ Future<Standalone<RangeResultRef>> GlobalConfigImpl::getRange(ReadYourWritesTran
                                                               KeyRangeRef kr) const {
 	Standalone<RangeResultRef> result;
 
-	// if (kr.begin != kr.end) {
-	// 	ryw->setSpecialKeySpaceErrorMsg("get range disabled, please fetch a single key");
-	// 	throw special_keys_api_failure();
-	// }
-
 	auto& globalConfig = GlobalConfig::globalConfig();
-	KeyRef key = kr.begin.removePrefix(getKeyRange().begin);
-	const std::any& any = globalConfig.get(key);
-	if (any.has_value()) {
-		if (any.type() == typeid(Standalone<StringRef>)) {
-			result.push_back_deep(result.arena(), KeyValueRef(kr.begin, std::any_cast<Standalone<StringRef>>(globalConfig.get(key)).contents()));
-		} else if (any.type() == typeid(int64_t)) {
-			result.push_back_deep(result.arena(), KeyValueRef(kr.begin, std::to_string(std::any_cast<int64_t>(globalConfig.get(key)))));
-		} else {
-			ASSERT(false);
+	KeyRangeRef modified = KeyRangeRef(kr.begin.removePrefix(getKeyRange().begin), kr.end.removePrefix(getKeyRange().begin));
+	std::map<KeyRef, std::any> values = globalConfig.get(modified);
+	for (const auto& [key, any] : values) {
+		Key prefixedKey = key.withPrefix(getKeyRange().begin);
+		if (any.has_value()) {
+			if (any.type() == typeid(Standalone<StringRef>)) {
+				result.push_back_deep(result.arena(), KeyValueRef(prefixedKey, std::any_cast<Standalone<StringRef>>(any).contents()));
+			} else if (any.type() == typeid(int64_t)) {
+				result.push_back_deep(result.arena(), KeyValueRef(prefixedKey, std::to_string(std::any_cast<int64_t>(any))));
+			} else if (any.type() == typeid(float)) {
+				result.push_back_deep(result.arena(), KeyValueRef(prefixedKey, std::to_string(std::any_cast<float>(any))));
+			} else if (any.type() == typeid(double)) {
+				result.push_back_deep(result.arena(), KeyValueRef(prefixedKey, std::to_string(std::any_cast<double>(any))));
+			} else {
+				ASSERT(false);
+			}
 		}
 	}
+
 	return result;
 }