Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mem leak in HashMap.keys_new_list #1547

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

konimarti
Copy link
Contributor

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)

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]>
@lerno
Copy link
Collaborator

lerno commented Oct 11, 2024

Great! Thank you

@lerno lerno merged commit 4e5ba32 into c3lang:master Oct 11, 2024
42 checks passed
@konimarti konimarti deleted the fix-map-mem-leak branch October 11, 2024 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants