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

os.args leaks on exit #1633

Closed
graphitemaster opened this issue Mar 18, 2022 · 4 comments
Closed

os.args leaks on exit #1633

graphitemaster opened this issue Mar 18, 2022 · 4 comments

Comments

@graphitemaster
Copy link
Contributor

graphitemaster commented Mar 18, 2022

Not really a bug per-se, since os.args should stick around the whole application, but it is annoying when using leak checking tools like valgrind to have instances of os.args appear as possible leaks. We already cleanup the runtime when returning from main and when calling os.exit, I propose freeing os.args inside there as well to help filter out these non-leak-leaks.

@gingerBill gingerBill changed the title os.argv leaks on exit os.args leaks on exit Mar 18, 2022
@gingerBill
Copy link
Member

gingerBill commented Mar 18, 2022

So we'd need to add something like the opposite of @(init) to the compiler since os.args is initialized before main.

@github-actions
Copy link

Hello!

I am marking this issue as stale as it has not received any engagement from the community or maintainers 120 days. That does not imply that the issue has no merit! If you feel strongly about this issue

  • open a PR referencing and resolving the issue;
  • leave a comment on it and discuss ideas how you could contribute towards resolving it;
  • leave a comment and describe in detail why this issue is critical for your use case;
  • open a new issue with updated details and a plan on resolving the issue.

The motivation for this automation is to help prioritize issues in the backlog and not ignore, reject, or belittle anyone..

@haesbaert
Copy link

haesbaert commented Jan 11, 2025

So we'd need to add something like the opposite of @(init) to the compiler since os.args is initialized before main.

Until then, how about freeing it in os.exit() for now, it catches most cases where the user aborts and can be added to the end of main.

EDIT: I noticed you implemented @(fini) in 94c1331, it's not documented in https://odin-lang.org/docs/overview/#procedure-attributes, is anything missing? It seems to work, I'll try my hand at a PR.

haesbaert added a commit to haesbaert/Odin that referenced this issue Jan 11, 2025

Verified

This commit was signed with the committer’s verified signature.
haesbaert Christiano Haesbaert
os.args is never freed, while this is an insignificant leak, it is a bit
annoying as it makes valgrind complain:

==234000== Memcheck, a memory error detector
==234000== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
==234000== Using Valgrind-3.24.0 and LibVEX; rerun with -h for copyright info
==234000== Command: ./wc /etc/passwd
==234000==
      41     104    2436 /etc/passwd
==234000==
==234000== HEAP SUMMARY:
==234000==     in use at exit: 47 bytes in 1 blocks
==234000==   total heap usage: 5 allocs, 4 frees, 4,195,875 bytes allocated
==234000==
==234000== 47 bytes in 1 blocks are possibly lost in loss record 1 of 1
==234000==    at 0x484BC13: calloc (vg_replace_malloc.c:1675)
==234000==    by 0x402E49: runtime._heap_alloc-932 (in /d/learn-odin/wc/wc)
==234000==    by 0x40A957: runtime.heap_alloc (in /d/learn-odin/wc/wc)
==234000==    by 0x436F2D: runtime.heap_allocator_proc.aligned_alloc-0 (in /d/learn-odin/wc/wc)
==234000==    by 0x4022DC: runtime.heap_allocator_proc (in /d/learn-odin/wc/wc)
==234000==    by 0x416590: runtime.make_aligned-24248 (in /d/learn-odin/wc/wc)
==234000==    by 0x41F710: runtime.make_slice-23725 (in /d/learn-odin/wc/wc)
==234000==    by 0x40156B: os._alloc_command_line_arguments-4731 (in /d/learn-odin/wc/wc)
==234000==    by 0x4011AF: __$startup_runtime (in /d/learn-odin/wc/wc)
==234000==    by 0x406F17: main (in /d/learn-odin/wc/wc)
==234000==
==234000== LEAK SUMMARY:
==234000==    definitely lost: 0 bytes in 0 blocks
==234000==    indirectly lost: 0 bytes in 0 blocks
==234000==      possibly lost: 47 bytes in 1 blocks
==234000==    still reachable: 0 bytes in 0 blocks
==234000==         suppressed: 0 bytes in 0 blocks
==234000==
==234000== For lists of detected and suppressed errors, rerun with: -s
==234000== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

With the fix the leak is gone, tested on linux only.
haesbaert added a commit to haesbaert/Odin that referenced this issue Jan 11, 2025

Verified

This commit was signed with the committer’s verified signature.
haesbaert Christiano Haesbaert
os.args is never freed, while this is an insignificant leak, it is a bit
annoying as it makes valgrind complain:

==234270== Memcheck, a memory error detector
==234270== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
==234270== Using Valgrind-3.24.0 and LibVEX; rerun with -h for copyright info
==234270== Command: ./wc /tmp/mulumulu
==234270==
       1       8      58 /tmp/mulumulu
==234270==
==234270== HEAP SUMMARY:
==234270==     in use at exit: 47 bytes in 1 blocks
==234270==   total heap usage: 5 allocs, 4 frees, 4,195,875 bytes allocated
==234270==
==234270== 47 bytes in 1 blocks are possibly lost in loss record 1 of 1
==234270==    at 0x484BC13: calloc (vg_replace_malloc.c:1675)
==234270==    by 0x402E49: runtime._heap_alloc-769 (in /d/learn-odin/wc/wc)
==234270==    by 0x40A8D7: runtime.heap_alloc (in /d/learn-odin/wc/wc)
==234270==    by 0x436E9D: runtime.heap_allocator_proc.aligned_alloc-0 (in /d/learn-odin/wc/wc)
==234270==    by 0x4022DC: runtime.heap_allocator_proc (in /d/learn-odin/wc/wc)
==234270==    by 0x4165E0: runtime.make_aligned-22560 (in /d/learn-odin/wc/wc)
==234270==    by 0x41F6D0: runtime.make_slice-22340 (in /d/learn-odin/wc/wc)
==234270==    by 0x40156B: os._alloc_command_line_arguments-4679 (in /d/learn-odin/wc/wc)
==234270==    by 0x4011AF: __$startup_runtime (in /d/learn-odin/wc/wc)
==234270==    by 0x406F17: main (in /d/learn-odin/wc/wc)
==234270==
==234270== LEAK SUMMARY:
==234270==    definitely lost: 0 bytes in 0 blocks
==234270==    indirectly lost: 0 bytes in 0 blocks
==234270==      possibly lost: 47 bytes in 1 blocks
==234270==    still reachable: 0 bytes in 0 blocks
==234270==         suppressed: 0 bytes in 0 blocks
==234270==
==234270== For lists of detected and suppressed errors, rerun with: -s
==234270== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

With the fix the leak is gone, tested on linux only.

While here, also make _alloc_command_line_arguments() private.
@haesbaert
Copy link

PR using fini #4680

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants