From f3dc3b97c79311681430f42dde321c5e3a951dba Mon Sep 17 00:00:00 2001 From: Koni Marti Date: Fri, 11 Oct 2024 13:25:04 +0200 Subject: [PATCH] Fix mem leak in HashMap.keys_new_list Fix a memory leak in HashMap.key_new_list(). The custom memory allocator will not be used, since key_new_list() will call HashMap.copy_keys() without passing the memory allocator along. Hence, HashMap.copy_keys() will allocate on the heap and these memory blocks will not be freed. To fix this, pass the custom allocator to HashMap.copy_keys(). Also, since HashMap.key_new_list() is deprecated anyways, replace it by HashMap.copy_keys(). Affected from this leak is Object.to_format() from std::collection::object (for an ObjectInternalMap) which is used in the JSON parser. The tests for the JSON parser show the memory leak: $ c3c compile-test test/unit/stdlib/encoding $ valgrind --leak-check=yes ./testrun ==1454708== ==1454708== HEAP SUMMARY: ==1454708== in use at exit: 384 bytes in 8 blocks ==1454708== total heap usage: 69 allocs, 61 frees, 528,672 bytes allocated ==1454708== ==1454708== 48 bytes in 1 blocks are definitely lost in loss record 1 of 8 ==1454708== at 0x48447A8: malloc (vg_replace_malloc.c:446) ==1454708== by 0x12CDBF: std.core.mem.allocator.LibcAllocator.acquire (libc_allocator.c3:42) ==1454708== by 0x1790FD: malloc_try (mem_allocator.c3:64) ==1454708== by 0x1790FD: alloc_array_try (mem_allocator.c3:286) ==1454708== by 0x1790FD: alloc_array (mem_allocator.c3:269) ==1454708== by 0x1790FD: copy_keys (hashmap.c3:310) ==1454708== by 0x1790FD: std_collections_map$String$p$std.collections.object.Object$.HashMap.key ==1454708== by 0x14D593: std.collections.object.Object.to_format (object.c3:53) ==1454708== by 0x164556: std.io.Formatter.print_with_function (formatter.c3:86) ==1454708== by 0x165B49: std.io.Formatter.out_str (formatter.c3:152) ==1454708== by 0x16E2B0: std.io.Formatter.vprintf (formatter.c3:456) ==1454708== by 0x12696B: std.core.dstring.DString.appendf (dstring.c3:532) ==1454708== by 0x124EA9: std.core.string.tformat (string.c3:79) ==1454708== by 0x113C79: json_test.test_string (json.c3:34) ==1454708== by 0x118AA1: std.core.runtime.run_tests (runtime.c3:227) ==1454708== by 0x1190B1: std.core.runtime.default_test_runner (runtime.c3:246) ==1454708== [..snip..] ==1454708== ==1454708== LEAK SUMMARY: ==1454708== definitely lost: 384 bytes in 8 blocks ==1454708== indirectly lost: 0 bytes in 0 blocks ==1454708== possibly lost: 0 bytes in 0 blocks ==1454708== still reachable: 0 bytes in 0 blocks ==1454708== suppressed: 0 bytes in 0 blocks ==1454708== ==1454708== For lists of detected and suppressed errors, rerun with: -s ==1454708== ERROR SUMMARY: 8 errors from 8 contexts (suppressed: 0 from 0) Signed-off-by: Koni Marti --- lib/std/collections/hashmap.c3 | 2 +- lib/std/collections/object.c3 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/std/collections/hashmap.c3 b/lib/std/collections/hashmap.c3 index df4a90db5..c085a6f8a 100644 --- a/lib/std/collections/hashmap.c3 +++ b/lib/std/collections/hashmap.c3 @@ -300,7 +300,7 @@ fn Key[] HashMap.key_tlist(&map) @deprecated("Use 'tcopy_keys'") **/ fn Key[] HashMap.key_new_list(&map, Allocator allocator = allocator::heap()) { - return map.copy_keys() @inline; + return map.copy_keys(allocator) @inline; } fn Key[] HashMap.copy_keys(&map, Allocator allocator = allocator::heap()) diff --git a/lib/std/collections/object.c3 b/lib/std/collections/object.c3 index 49a952370..89662cddc 100644 --- a/lib/std/collections/object.c3 +++ b/lib/std/collections/object.c3 @@ -50,7 +50,7 @@ fn usz! Object.to_format(&self, Formatter* formatter) @dynamic usz n = formatter.printf("{")!; @stack_mem(1024; Allocator mem) { - foreach (i, key : self.map.key_new_list(mem)) + foreach (i, key : self.map.copy_keys(mem)) { if (i > 0) n += formatter.printf(",")!; n += formatter.printf(`"%s":`, key)!;