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 Valgrind memory leak warning by freeing alternate signal stack #15336

Closed
wants to merge 4 commits into from

Conversation

kojix2
Copy link
Contributor

@kojix2 kojix2 commented Jan 11, 2025

Hello,

This is a proof-of-concept pull request.

Recently, it was reported on the forum that when running Crystal programs with Valgrind, the alternate signal stack allocated by LibC.malloc is not freed before the program ends. Instead, the operating system frees it after the program finishes.

https://forum.crystal-lang.org/t/valgrind-memory-leak-in-helloworld-app-crystal-lang/7583

emrum

==387674== HEAP SUMMARY:
==387674== in use at exit: 8,192 bytes in 1 blocks
==387674== total heap usage: 15 allocs, 14 frees, 12,336 bytes allocated

HertzDevil

For the record, the 8192 bytes leaked here are the alternate stack for the signal handler, which is allocated using LibC.malloc directly

To fix this, I added an at_exit signal handler that switches back to the default stack and then frees the alternate stack. This stops Valgrind from showing a "memory leak" warning.

This pull request also adds a new system-level at_exit handler in AtExitHandlers. I created this because it is not good practice to use the regular at_exit for system tasks. The new handler can be used to free memory allocated by LibC.malloc and handle other system cleanup.

Thank you for reviewing this.

@kojix2 kojix2 closed this Jan 11, 2025
@kojix2
Copy link
Contributor Author

kojix2 commented Jan 11, 2025

hello.cr

puts "hello world"

Before

git checkout master
bin/crystal build ./hello.cr
valgrind ./hello
==122981== HEAP SUMMARY:
==122981==     in use at exit: 8,192 bytes in 1 blocks
==122981==   total heap usage: 15 allocs, 14 frees, 12,336 bytes allocated
==122981== 
==122981== LEAK SUMMARY:
==122981==    definitely lost: 8,192 bytes in 1 blocks
==122981==    indirectly lost: 0 bytes in 0 blocks
==122981==      possibly lost: 0 bytes in 0 blocks
==122981==    still reachable: 0 bytes in 0 blocks
==122981==         suppressed: 0 bytes in 0 blocks
==122981== Rerun with --leak-check=full to see details of leaked memory

After

git checkout altstack
bin/crystal build ./hello.cr
valgrind ./hello
==123551== HEAP SUMMARY:
==123551==     in use at exit: 0 bytes in 0 blocks
==123551==   total heap usage: 15 allocs, 15 frees, 12,336 bytes allocated
==123551== 
==123551== All heap blocks were freed -- no leaks are possible

@crysbot
Copy link

crysbot commented Jan 11, 2025

This pull request has been mentioned on Crystal Forum. There might be relevant details there:

https://forum.crystal-lang.org/t/valgrind-memory-leak-in-helloworld-app-crystal-lang/7583/13

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