From 41f95a7423ae00a81d3cb4df81bc975ac3a66f00 Mon Sep 17 00:00:00 2001 From: Anton Tarasenko Date: Sat, 24 Jul 2021 08:41:25 +0700 Subject: [PATCH] Fix bug with deallocating `AssociativeArray`s keys When key was deallocated, `AssociativeArray` attempted to free values using that key's life version. --- sources/Data/Collections/AssociativeArray.uc | 2 +- .../Tests/TEST_AssociativeArray.uc | 43 ++++++++++++++++++- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/sources/Data/Collections/AssociativeArray.uc b/sources/Data/Collections/AssociativeArray.uc index d01ecad..dffba1b 100644 --- a/sources/Data/Collections/AssociativeArray.uc +++ b/sources/Data/Collections/AssociativeArray.uc @@ -173,7 +173,7 @@ private final function CleanBucket(out Bucket bucketToClean) if (nextEntry.key.GetLifeVersion() != nextEntry.keyLifeVersion) { if (bucketEntries[i].value != none && bucketEntries[i].managed) { - bucketEntries[i].value.FreeSelf(nextEntry.keyLifeVersion); + bucketEntries[i].value.FreeSelf(nextEntry.valueLifeVersion); } bucketEntries.Remove(i, 1); // We'll update the count, but won't trigger hash table size update diff --git a/sources/Data/Collections/Tests/TEST_AssociativeArray.uc b/sources/Data/Collections/Tests/TEST_AssociativeArray.uc index 80a02e0..ccbf77a 100644 --- a/sources/Data/Collections/Tests/TEST_AssociativeArray.uc +++ b/sources/Data/Collections/Tests/TEST_AssociativeArray.uc @@ -281,6 +281,47 @@ protected static function Test_Managed() } protected static function Test_DeallocationHandling() +{ + Context("Testing how `AssociativeArray` deals with external deallocation of" + @ "keys and managed objects."); + SubTest_DeallocationHandlingKeys(); + SubTest_DeallocationHandlingManagedObjects(); +} + +protected static function SubTest_DeallocationHandlingKeys() +{ + local IntBox key1; + local BoolBox key2; + local Text key3; + local AssociativeArray array; + local array keys; + key1 = __().box.int(3881); + key2 = __().box.bool(true); + key3 = __().text.FromString("Text key, bay bee"); + array = AssociativeArray(__().memory.Allocate(class'AssociativeArray')); + class'MockItem'.default.objectCount = 0; + array.SetItem(key1, NewMockItem()); + array.SetItem(key2, __().box.float(32.31), true); + array.SetItem(key3, NewMockItem(), true); + + Issue("Deallocating keys does not remove them from `AssociativeArray`."); + key1.FreeSelf(); + key3.FreeSelf(); + keys = array.GetKeys(); + TEST_ExpectTrue(keys.length == 1); + TEST_ExpectTrue(keys[0] == key2); + + Issue("`AssociativeArray` does not deallocate managed objects, even though" + @ "their keys were deallocated"); + TEST_ExpectTrue(class'MockItem'.default.objectCount < 2); + TEST_ExpectNone(array.GetItem(__().box.int(3881))); + + Issue("`AssociativeArray` deallocates unmanaged objects, when" + @ "their keys were deallocated"); + TEST_ExpectTrue(class'MockItem'.default.objectCount > 0); +} + +protected static function SubTest_DeallocationHandlingManagedObjects() { local MockItem exampleItem; local AssociativeArray array; @@ -292,8 +333,6 @@ protected static function Test_DeallocationHandling() array.SetItem(__().box.int(23), NewMockItem()); array.SetItem(__().box.int(242), NewMockItem(), true); array.CreateItem(__().box.int(24532), class'MockItem'); - Context("Testing how `AssociativeArray` deals with external deallocation of" - @ "managed objects."); Issue("`AssociativeArray` does not return `none` even though stored object" @ "was already deallocated."); array.GetItem(__().box.int(23)).FreeSelf();