Skip to content

Commit

Permalink
Fix mem leak in HashMap.keys_new_list
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
konimarti committed Oct 11, 2024
1 parent efeb9e6 commit f3dc3b9
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion lib/std/collections/hashmap.c3
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion lib/std/collections/object.c3
Original file line number Diff line number Diff line change
Expand Up @@ -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)!;
Expand Down

0 comments on commit f3dc3b9

Please sign in to comment.