fix Dictionary and DictionaryIterator memory leaks

# Conflicts:
#	src/version.h

src/dictionary.cpp

@@ -3,20 +3,87 @@
 #include "scripting/vm/vm.h"
 #include "serializer.h"
 
+#include <cassert>
+
+//=====================================================================================
+//
+// DObject implementations for Dictionary and DictionaryIterator
+//
+//=====================================================================================
+
+IMPLEMENT_CLASS(Dictionary, false, false);
+
+IMPLEMENT_CLASS(DictionaryIterator, false, true);
+
+IMPLEMENT_POINTERS_START(DictionaryIterator)
+IMPLEMENT_POINTER(Dict)
+IMPLEMENT_POINTERS_END
+
+//=====================================================================================
+//
+// Dictionary functions
+//
+//=====================================================================================
+
+void Dictionary::Serialize(FSerializer &arc)
+{
+	Super::Serialize(arc);
+
+	constexpr char key[] { "dictionary" };
+
+	if (arc.isWriting())
+	{
+		// Pass this instance to serializer.
+		Dictionary *pointerToThis = this;
+		arc(key, pointerToThis);
+	}
+	else
+	{
+		// Receive new Dictionary, copy contents, clean up.
+		Dictionary *pointerToDeserializedDictionary;
+		arc(key, pointerToDeserializedDictionary);
+		TransferFrom(*pointerToDeserializedDictionary);
+		delete pointerToDeserializedDictionary;
+	}
+}
+
+static Dictionary *DictCreate()
+{
+	Dictionary *dict { Create<Dictionary>() };
+
+	return dict;
+}
+
+static void DictInsert(Dictionary *dict, const FString &key, const FString &value)
+{
+	dict->Insert(key, value);
+}
+
+static void DictAt(const Dictionary *dict, const FString &key, FString *result)
+{
+	const FString *value = dict->CheckKey(key);
+	*result = value ? *value : "";
+}
+
+static void DictToString(const Dictionary *dict, FString *result)
+{
+	*result = DictionaryToString(*dict);
+}
+
+static void DictRemove(Dictionary *dict, const FString &key)
+{
+	dict->Remove(key);
+}
+
 //=====================================================================================
 //
 // Dictionary exports
 //
 //=====================================================================================
 
-DEFINE_ACTION_FUNCTION(_Dictionary, Create)
+DEFINE_ACTION_FUNCTION_NATIVE(_Dictionary, Create, DictCreate)
 {
-	ACTION_RETURN_POINTER(new Dictionary);
-}
-
-static void DictInsert(Dictionary *dict, const FString &key, const FString &value)
-{
-	dict->Insert(key, value);
+	ACTION_RETURN_POINTER(DictCreate());
 }
 
 DEFINE_ACTION_FUNCTION_NATIVE(_Dictionary, Insert, DictInsert)
@@ -28,12 +95,6 @@ DEFINE_ACTION_FUNCTION_NATIVE(_Dictionary, Insert, DictInsert)
 	return 0;
 }
 
-static void DictAt(const Dictionary *dict, const FString &key, FString *result)
-{
-	const FString *value = dict->CheckKey(key);
-	*result = value ? *value : "";
-}
-
 DEFINE_ACTION_FUNCTION_NATIVE(_Dictionary, At, DictAt)
 {
 	PARAM_SELF_STRUCT_PROLOGUE(Dictionary);
@@ -43,11 +104,6 @@ DEFINE_ACTION_FUNCTION_NATIVE(_Dictionary, At, DictAt)
 	ACTION_RETURN_STRING(result);
 }
 
-static void DictToString(const Dictionary *dict, FString *result)
-{
-	*result = DictionaryToString(*dict);
-}
-
 DEFINE_ACTION_FUNCTION_NATIVE(_Dictionary, ToString, DictToString)
 {
 	PARAM_SELF_STRUCT_PROLOGUE(Dictionary);
@@ -56,21 +112,11 @@ DEFINE_ACTION_FUNCTION_NATIVE(_Dictionary, ToString, DictToString)
 	ACTION_RETURN_STRING(result);
 }
 
-static Dictionary *DictFromString(const FString& string)
-{
-	return DictionaryFromString(string);
-}
-
-DEFINE_ACTION_FUNCTION_NATIVE(_Dictionary, FromString, DictFromString)
+DEFINE_ACTION_FUNCTION_NATIVE(_Dictionary, FromString, DictionaryFromString)
 {
 	PARAM_PROLOGUE;
 	PARAM_STRING(string);
-	ACTION_RETURN_POINTER(DictFromString(string));
-}
-
-static void DictRemove(Dictionary *dict, const FString &key)
-{
-	dict->Remove(key);
+	ACTION_RETURN_POINTER(DictionaryFromString(string));
 }
 
 DEFINE_ACTION_FUNCTION_NATIVE(_Dictionary, Remove, DictRemove)
@@ -83,21 +129,65 @@ DEFINE_ACTION_FUNCTION_NATIVE(_Dictionary, Remove, DictRemove)
 
 //=====================================================================================
 //
-// DictionaryIterator exports
+// DictionaryIterator functions
 //
 //=====================================================================================
 
-DictionaryIterator::DictionaryIterator(const Dictionary &dict)
-	: Iterator(dict)
+DictionaryIterator::DictionaryIterator()
+	: Iterator(nullptr)
 	, Pair(nullptr)
+	, Dict(nullptr)
 {
 }
 
-static DictionaryIterator *DictIteratorCreate(const Dictionary *dict)
+void DictionaryIterator::Serialize(FSerializer &arc)
 {
-	return new DictionaryIterator(*dict);
+	if (arc.isWriting())
+	{
+		I_Error("Attempt to save pointer to unhandled type DictionaryIterator");
+	}
 }
 
+void DictionaryIterator::init(Dictionary *dict)
+{
+	Iterator = std::make_unique<Dictionary::ConstIterator>(*dict);
+	Dict = dict;
+
+	GC::WriteBarrier(this, Dict);
+}
+
+static DictionaryIterator *DictIteratorCreate(Dictionary *dict)
+{
+	DictionaryIterator *iterator = Create<DictionaryIterator>();
+	iterator->init(dict);
+
+	return iterator;
+}
+
+static int DictIteratorNext(DictionaryIterator *self)
+{
+	assert(self->Iterator != nullptr);
+
+	const bool hasNext { self->Iterator->NextPair(self->Pair) };
+	return hasNext;
+}
+
+static void DictIteratorKey(const DictionaryIterator *self, FString *result)
+{
+	*result = self->Pair ? self->Pair->Key : FString {};
+}
+
+static void DictIteratorValue(const DictionaryIterator *self, FString *result)
+{
+	*result = self->Pair ? self->Pair->Value : FString {};
+}
+
+//=====================================================================================
+//
+// DictionaryIterator exports
+//
+//=====================================================================================
+
 DEFINE_ACTION_FUNCTION_NATIVE(_DictionaryIterator, Create, DictIteratorCreate)
 {
 	PARAM_PROLOGUE;
@@ -105,22 +195,12 @@ DEFINE_ACTION_FUNCTION_NATIVE(_DictionaryIterator, Create, DictIteratorCreate)
 	ACTION_RETURN_POINTER(DictIteratorCreate(dict));
 }
 
-static int DictIteratorNext(DictionaryIterator *self)
-{
-	return self->Iterator.NextPair(self->Pair);
-}
-
 DEFINE_ACTION_FUNCTION_NATIVE(_DictionaryIterator, Next, DictIteratorNext)
 {
 	PARAM_SELF_STRUCT_PROLOGUE(DictionaryIterator);
 	ACTION_RETURN_BOOL(DictIteratorNext(self));
 }
 
-static void DictIteratorKey(const DictionaryIterator *self, FString *result)
-{
-	*result = self->Pair ? self->Pair->Key : FString {};
-}
-
 DEFINE_ACTION_FUNCTION_NATIVE(_DictionaryIterator, Key, DictIteratorKey)
 {
 	PARAM_SELF_STRUCT_PROLOGUE(DictionaryIterator);
@@ -129,11 +209,6 @@ DEFINE_ACTION_FUNCTION_NATIVE(_DictionaryIterator, Key, DictIteratorKey)
 	ACTION_RETURN_STRING(result);
 }
 
-static void DictIteratorValue(const DictionaryIterator *self, FString *result)
-{
-	*result = self->Pair ? self->Pair->Value : FString {};
-}
-
 DEFINE_ACTION_FUNCTION_NATIVE(_DictionaryIterator, Value, DictIteratorValue)
 {
 	PARAM_SELF_STRUCT_PROLOGUE(DictionaryIterator);

src/dictionary.h

@@ -2,13 +2,63 @@
 
 #include "tarray.h"
 #include "zstring.h"
+#include "dobject.h"
 
-using Dictionary = TMap<FString, FString>;
+#include <memory>
 
-struct DictionaryIterator
+/**
+ * @brief The Dictionary class exists to be exported to ZScript.
+ *
+ * It is a string-to-string map.
+ *
+ * It is derived from DObject to be a part of normal GC process.
+ */
+class Dictionary final : public DObject, public TMap<FString, FString>
 {
-	explicit DictionaryIterator(const Dictionary &dict);
+	DECLARE_CLASS(Dictionary, DObject)
 
-	Dictionary::ConstIterator Iterator;
-	Dictionary::ConstPair *Pair;
+public:
+
+	void Serialize(FSerializer &arc) override;
+};
+
+/**
+ * @brief The DictionaryIterator class exists to be exported to ZScript.
+ *
+ * It provides iterating over a Dictionary. The order is not specified.
+ *
+ * It is derived from DObject to be a part of normal GC process.
+ */
+class DictionaryIterator final : public DObject
+{
+	DECLARE_CLASS(DictionaryIterator, DObject)
+	HAS_OBJECT_POINTERS
+
+public:
+
+	~DictionaryIterator() override = default;
+
+	/**
+	 * IMPLEMENT_CLASS macro needs a constructor without parameters.
+	 *
+	 * @see init().
+	 */
+	DictionaryIterator();
+
+    void Serialize(FSerializer &arc) override;
+
+	/**
+	 * @brief init function complements constructor.
+	 * @attention always call init after constructing DictionaryIterator.
+	 */
+	void init(Dictionary *dict);
+
+	std::unique_ptr<Dictionary::ConstIterator> Iterator;
+	Dictionary::ConstPair *Pair;
+
+	/**
+	 * @brief Dictionary attribute exists for holding a pointer to iterated
+	 * dictionary, so it is known by GC.
+	 */
+	Dictionary *Dict;
 };

src/scripting/thingdef_data.cpp

@@ -851,21 +851,6 @@ void InitThingdef()
 	wbplayerstruct->Size = sizeof(wbplayerstruct_t);
 	wbplayerstruct->Align = alignof(wbplayerstruct_t);
 
-	auto dictionarystruct = NewStruct("Dictionary", nullptr, true);
-	dictionarystruct->Size = sizeof(Dictionary);
-	dictionarystruct->Align = alignof(Dictionary);
-	NewPointer(dictionarystruct, false)->InstallHandlers(
-		[](FSerializer &ar, const char *key, const void *addr)
-		{
-			ar(key, *(Dictionary **)addr);
-		},
-		[](FSerializer &ar, const char *key, void *addr)
-		{
-			Serialize<Dictionary>(ar, key, *(Dictionary **)addr, nullptr);
-			return true;
-		}
-	);
-
 	FAutoSegIterator probe(CRegHead, CRegTail);
 
 	while (*++probe != NULL)

src/serializer.cpp

@@ -2134,7 +2134,7 @@ Dictionary *DictionaryFromString(const FString &string)
 		return nullptr;
 	}
 
-	Dictionary *const dict = new Dictionary;
+	Dictionary *const dict = Create<Dictionary>();
 
 	if (string.IsEmpty())
 	{

src/version.h

@@ -79,11 +79,11 @@ const char *GetVersionString();
 #define SAVEGAME_EXT "zds"
 
 // MINSAVEVER is the minimum level snapshot version that can be loaded.
-#define MINSAVEVER	4555
+#define MINSAVEVER 4557
 
 // Use 4500 as the base git save version, since it's higher than the
 // SVN revision ever got.
-#define SAVEVER 4556
+#define SAVEVER 4557
 
 // This is so that derivates can use the same savegame versions without worrying about engine compatibility
 #define GAMESIG "LZDOOM"

wadsrc/static/zscript/dictionary.zs

@@ -1,4 +1,11 @@
-struct Dictionary native
+/**
+ * Dictionary provides key-value storage.
+ *
+ * Both keys and values are strings.
+ *
+ * @note keys are case-sensitive.
+ */
+class Dictionary
 {
 	native static Dictionary Create();
 
@@ -23,7 +30,15 @@ struct Dictionary native
 	native String ToString() const;
 }
 
-struct DictionaryIterator native
+/**
+ * Provides iterating over a Dictionary.
+ *
+ * Order is not specified.
+ *
+ * DictionaryIterator is not serializable. To make DictionaryIterator a class
+ * member, use `transient` keyword.
+ */
+class DictionaryIterator
 {
 	native static DictionaryIterator Create(Dictionary dict);