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

Memory leak when unloading SF3 #528

Closed
derselbst opened this issue Apr 15, 2019 · 6 comments
Closed

Memory leak when unloading SF3 #528

derselbst opened this issue Apr 15, 2019 · 6 comments
Labels
Milestone

Comments

@derselbst
Copy link
Member

FluidSynth version

2.0.4 and trunk

Current behavior

explicit unloading of a sf3 does not release memory

Other information

valgrind says:

==9970== 37,536 bytes in 2,346 blocks are still reachable in loss record 189 of 192
==9970==    at 0x4C2E01F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==9970==    by 0x4E532E8: new_fluid_list (fluid_list.c:37)
==9970==    by 0x4E533CD: fluid_list_prepend (fluid_list.c:91)
==9970==    by 0x4E5FE4E: fluid_samplecache_load (fluid_samplecache.c:100)
==9970==    by 0x4E57DD5: fluid_defsfont_load_sampledata (fluid_defsfont.c:312)
==9970==    by 0x4E57F84: fluid_defsfont_load_all_sampledata (fluid_defsfont.c:379)
==9970==    by 0x4E58236: fluid_defsfont_load (fluid_defsfont.c:478)
==9970==    by 0x4E57890: fluid_defsfloader_load (fluid_defsfont.c:105)
==9970==    by 0x4E74EF4: fluid_synth_sfload (fluid_synth.c:4503)
==9970==    by 0x403328: main (fluidsynth.c:771)
==9970== 
==9970== 79,343 bytes in 2,346 blocks are still reachable in loss record 190 of 192
==9970==    at 0x4C2E01F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==9970==    by 0x4E60139: new_samplecache_entry (fluid_samplecache.c:205)
==9970==    by 0x4E5FE21: fluid_samplecache_load (fluid_samplecache.c:92)
==9970==    by 0x4E57DD5: fluid_defsfont_load_sampledata (fluid_defsfont.c:312)
==9970==    by 0x4E57F84: fluid_defsfont_load_all_sampledata (fluid_defsfont.c:379)
==9970==    by 0x4E58236: fluid_defsfont_load (fluid_defsfont.c:478)
==9970==    by 0x4E57890: fluid_defsfloader_load (fluid_defsfont.c:105)
==9970==    by 0x4E74EF4: fluid_synth_sfload (fluid_synth.c:4503)
==9970==    by 0x403328: main (fluidsynth.c:771)
==9970== 
==9970== 187,680 bytes in 2,346 blocks are still reachable in loss record 191 of 192
==9970==    at 0x4C2E01F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==9970==    by 0x4E600D4: new_samplecache_entry (fluid_samplecache.c:195)
==9970==    by 0x4E5FE21: fluid_samplecache_load (fluid_samplecache.c:92)
==9970==    by 0x4E57DD5: fluid_defsfont_load_sampledata (fluid_defsfont.c:312)
==9970==    by 0x4E57F84: fluid_defsfont_load_all_sampledata (fluid_defsfont.c:379)
==9970==    by 0x4E58236: fluid_defsfont_load (fluid_defsfont.c:478)
==9970==    by 0x4E57890: fluid_defsfloader_load (fluid_defsfont.c:105)
==9970==    by 0x4E74EF4: fluid_synth_sfload (fluid_synth.c:4503)
==9970==    by 0x403328: main (fluidsynth.c:771)
==9970== 
==9970== 342,593,540 bytes in 2,346 blocks are still reachable in loss record 192 of 192
==9970==    at 0x4C2E01F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==9970==    by 0x4E5FCD6: fluid_sffile_read_vorbis (fluid_sffile.c:2618)
==9970==    by 0x4E5B69D: fluid_sffile_read_sample_data (fluid_sffile.c:489)
==9970==    by 0x4E60202: new_samplecache_entry (fluid_samplecache.c:222)
==9970==    by 0x4E5FE21: fluid_samplecache_load (fluid_samplecache.c:92)
==9970==    by 0x4E57DD5: fluid_defsfont_load_sampledata (fluid_defsfont.c:312)
==9970==    by 0x4E57F84: fluid_defsfont_load_all_sampledata (fluid_defsfont.c:379)
==9970==    by 0x4E58236: fluid_defsfont_load (fluid_defsfont.c:478)
==9970==    by 0x4E57890: fluid_defsfloader_load (fluid_defsfont.c:105)
==9970==    by 0x4E74EF4: fluid_synth_sfload (fluid_synth.c:4503)
==9970==    by 0x403328: main (fluidsynth.c:771)
==9970== 
==9970== LEAK SUMMARY:
==9970==    definitely lost: 48 bytes in 1 blocks
==9970==    indirectly lost: 195 bytes in 9 blocks
==9970==      possibly lost: 42,626 bytes in 1,300 blocks
==9970==    still reachable: 342,964,202 bytes in 9,545 blocks
==9970==         suppressed: 0 bytes in 0 blocks
==9970== 

@derselbst derselbst added the bug label Apr 15, 2019
@derselbst derselbst added this to the 2.0-post milestone Apr 15, 2019
@ReinholdH
Copy link

Just an additional comment:
For sf3 soundfont each individual sample is loaded individually in fluid_defsfont_load_all_sampledata. But when unloading a sf3 file, just the large sample block is deleted from the samplecache like for a sf2 soundfont. The solution has to be - similar to loading - that each sample needs to be unloaded individually from the samplecache and the memory of each individual sample needs to be released with FLUID_FREE.

Based on fluidsynth 2.0.1 I was only able to create a quick and dirty fix which I sent to "derselbst". The fix is in fluid_defsfont.h, fluid_defsfont.c. The final solution needs to be more verbose because unloading of a sf3 file is different now than unloading of a sf2 file. The information "if(sf3_file)" needs to be preserved from loading to unloading. I recommend not to open the sf3 file again just for this information but save this information accordingly.

@mawe42
Copy link
Member

mawe42 commented Apr 16, 2019

We could unload the individual samples in delete_fluid_defsfont if their sample->data pointer is different to the defsfont->sampledata pointer. For bulk loaded SF2, they are always identical, for SF3 they always differ.

@mawe42
Copy link
Member

mawe42 commented Apr 16, 2019

So basically something like this should already be enough:

diff --git a/src/sfloader/fluid_defsfont.c b/src/sfloader/fluid_defsfont.c
index ae8a9fb2..0a812770 100644
--- a/src/sfloader/fluid_defsfont.c
+++ b/src/sfloader/fluid_defsfont.c
@@ -246,7 +246,16 @@ int delete_fluid_defsfont(fluid_defsfont_t *defsfont)
 
     for(list = defsfont->sample; list; list = fluid_list_next(list))
     {
-        delete_fluid_sample((fluid_sample_t *) fluid_list_get(list));
+        sample = (fluid_sample_t *) fluid_list_get(list);
+
+        /* If the sample data pointer is different to the sampledata chunk of
+         * the soundfont, then the sample has been loaded individually (SF3)
+         * and needs to be unloaded explicitly. */
+        if (sample->data && sample->data != defsfont->sampledata)
+        {
+            fluid_samplecache_unload(sample->data);
+        }
+        delete_fluid_sample(sample);
     }
 
     if(defsfont->sample)

@mawe42
Copy link
Member

mawe42 commented Apr 16, 2019

Added a PR that implements this idea. A check with valgrind shows that it actually fixes the memory-leak.

@ReinholdH
Copy link

I tested the fix in our app and it solves the problem. Thank you.

@derselbst
Copy link
Member Author

Thank you @ReinholdH for the report and @mawe42 for the fix!

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

Successfully merging a pull request may close this issue.

3 participants