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

ALSA: Free global configuration tree after each snd_ctl_close and snd… #448

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

mttjcksn
Copy link
Contributor

In reference to issue #447:

This is a PR to update the ALSA backend to free the global configuration tree after each successful snd_*_open() (so actually after each snd_*_close()) in order to prevent memory leaks as described here.

To show the issue, valgrand memcheck was run on the test apps with following commands (RtAudio commit 7aba116):

valgrind --log-file=probe-mem.log --leak-check=full --show-leak-kinds=definite,indirect,possible --show-below-main=yes --dsymutil=no --leak-resolution=high --partial-loads-ok=no --track-origins=yes ./audioprobe
valgrind --log-file=record-mem.log --leak-check=full --show-leak-kinds=definite,indirect,possible --show-below-main=yes --dsymutil=no --leak-resolution=high --partial-loads-ok=no --track-origins=yes ./record 1 44100 8 0 0
valgrind --log-file=play-mem.log --leak-check=full --show-leak-kinds=definite,indirect,possible --show-below-main=yes --dsymutil=no --leak-resolution=high --partial-loads-ok=no --track-origins=yes ./playraw 1 44100 record.raw 0 0
valgrind --log-file=playsaw-mem.log --leak-check=full --show-leak-kinds=definite,indirect,possible --show-below-main=yes --dsymutil=no --leak-resolution=high --partial-loads-ok=no --track-origins=yes ./playsaw 2 48000 0 0 2
valgrind --log-file=testall-mem.log --leak-check=full --show-leak-kinds=definite,indirect,possible --show-below-main=yes --dsymutil=no --leak-resolution=high --partial-loads-ok=no --track-origins=yes ./testall 2 44100 0 0 0 0 
valgrind --log-file=teststops-mem.log --leak-check=full --show-leak-kinds=definite,indirect,possible --show-below-main=yes --dsymutil=no --leak-resolution=high --partial-loads-ok=no --track-origins=yes ./teststops 2 16000 0 0 0 0 

For all of these test apps, memcheck reports 118 errors, with 832 bytes definitely lost, 69,729 bytes possibly lost.

Following the patch, all report 0 erros and 0 bytes lost, except teststops, which reports 1 error of 768 bytes definitely lost. Perhaps another ALSA issue to investigate.

All pre and post-fix logs are attached. The post-fix logs are appended with -fork.
logs.zip

@garyscavone garyscavone merged commit 17d6a6d into thestk:master Dec 3, 2024
12 checks passed
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