GNOME Bugzilla – Bug 551075
Dnd a image file from webpage to nautilus desktop -> memory leak happens in gvfsd-http
Last modified: 2009-01-21 14:11:43 UTC
Filed against gvfs 0.99.5 in Fedora: Description of problem: Dnd a image from file webpage to nautilus desktop -> memory leak happens in gvfsd-http About 800kB memory leak per Dnd. 4316 is gvfsd-http process number. $ pmap -x 4316 | grep total total kB 37532 - - - $ pmap -x 4316 | grep total total kB 38356 - - - $ pmap -x 4316 | grep total total kB 39304 - - - $ pmap -x 4316 | grep total total kB 40076 - - - $ pmap -x 4316 | grep total total kB 40784 - - - $ pmap -x 4316 | grep total total kB 41492 - - - $ pmap -x 4316 | grep total total kB 42432 - - - $ pmap -x 4316 | grep total total kB 43132 $ pmap -x 4316 | grep total total kB 43844 - - $ pmap -x 4316 | grep total total kB 44628 $ pmap -x 4316 | grep total total kB 45492 Version-Release number of selected component (if applicable): 0.99.6-1.fc10 How reproducible: always Steps to Reproduce: 1. Dnd a image file from webpage (like http://fedoraproject.org/w/uploads/0/08/Artwork_ArtTeamProjects_FaceBrowser_CurrentPics.png )
Same leak also happens when you download podcasts in Rhythmbox, once a patch has been proposed for this bug, verify that these bugs no longer trigger the memory leak: 1. start rhythmbox 2. select "Music::New podcast feed" 3. type in the URL: http://www.sr.se/rssfeed/rssfeed.aspx?Poddfeed=4891 4. select the "podcasts" item in the listview on the left 5. download some podcasts by right-clicking on them and selecting download Note: depending on which version of Rhythmbox that is used for the repro it might be necessary to download the podcasts one by one due to this crash bug (fixed in trunk): http://bugzilla.gnome.org/show_bug.cgi?id=561105
The following bug report in Ubuntu launchpad was also filed for this memory leak: https://bugs.launchpad.net/ubuntu/+source/gvfs/+bug/225615
Created attachment 123069 [details] Small test program to download files Attached is a program that downloads a file using gvfs. Compile with gcc `pkg-config --cflags --libs gio-2.0` gio-file-copy.c -o gio-file-copy I can see the memory growing by downloading "small" files of 50-100KB. No need to download something bigger ;-) I don't think it can be used in a useful way to solve the problem... just providing a "more" stupid test case.
I bet on a ref counting problem inside of soup-stream. I really need to look at it.
Christian, if I want to restart gvfsd-http under valgrind or gdb, how can I do that? I read this reply by Alexander: http://www.nabble.com/Debugging-gvfsd-td15544318.html So I tried just typing blindly "./gvfsd-http host=foo" but I guess foo should be replaced with something else?
This is how I do it: Open a terminal (and make sure you have no prior gvfs-httpd running) and run: valgrind --show-reachable=yes --leak-check=full /usr/lib/gvfs/gvfsd-http --spawner :1.5 /org/gtk/gvfs/exec_spaw/3 > /dev/null Then in another terminal run: gvfs-copy http://blah . where 'blah' is replaced of course.
Sorry, I forgot to add some results. Probably the most interesting trace with --leak-check=full is: ==28676== 3,511,888 bytes in 4,251 blocks are possibly lost in loss record 45 of 46 ==28676== at 0x4C241D0: memalign (vg_replace_malloc.c:460) ==28676== by 0x4C2428A: posix_memalign (vg_replace_malloc.c:569) ==28676== by 0x61F3F20: slab_allocator_alloc_chunk (gslice.c:1136) ==28676== by 0x61F50E2: g_slice_alloc (gslice.c:666) ==28676== by 0x5D76F84: g_signal_emit_valist (gsignal.c:2940) ==28676== by 0x5D77B32: g_signal_emit (gsignal.c:3034) ==28676== by 0x58BE82B: read_body_chunk (soup-message-io.c:313) ==28676== by 0x58BECA7: io_read (soup-message-io.c:754) ==28676== by 0x58BF66C: io_unpause_internal (soup-message-io.c:955) ==28676== by 0x61D6D3A: g_main_context_dispatch (gmain.c:2144) ==28676== by 0x61DA50C: g_main_context_iterate (gmain.c:2778) ==28676== by 0x61DAA3C: g_main_loop_run (gmain.c:2986) And if you wait for more of the file the transfer this just grows and grows. Is there meant to be a file which the data is flushed out to periodically? Or is this meant to read the entire file into a buffer? I'm using Ubuntu 8.10 if that helps narrowing down the line number references for the source code.
You need to run with "G_SLICE=always-malloc" set in the environment if you want to use valgrind. (The glib slice allocator doesn't free() its memory, it just keeps track of it itself so that it can reuse it very quickly later. But to valgrind that looks like a leak.)
Created attachment 124374 [details] [review] Patch that sets accumulate to false on soup soup message body Hi, I have been investigating this problem a bit and found that SOUP by default accumulates all the received data on the response_body by default, unless you disable it by using the function soup_message_body_set_accumulate. I have attached a patch that disables the accumulation, and now gvfsd-http's memory usage does not explode when I fetch large files. I still think there is another leak somewhere because the memory usage still grows, but much slower than before.
I've applied you patch to the version that ships in gvfs-backends of ubuntu jaunty and it leaks significantly less (and no regressions as far as I can see). The leak rate after applying this patch seems to be about 10-15% of the downloaded file size so I still think it's worth hunting the rest of the leak (even though I hope this patch can be applied right away of course). Christian Kellner mentioned that he's worked on rewriting parts of this code. I've not heard any specifics about the status of that work though.
Created attachment 124380 [details] gvfs copying a 4MB .ogg under valgrind without peter's patch
Created attachment 124381 [details] gvfs copying same 4MB .ogg under valgrind WITH peter's patch
(In reply to comment #9) > I have attached a patch that disables the accumulation, and now gvfsd-http's > memory usage does not explode when I fetch large files. That memory should all be freed when the message is destroyed. So if disabling accumulate results in less memory leaked, then that implies that the SoupMessage must not be getting destroyed... (Your patch is still correct though; even if we fixed the leak, leaving accumulate enabled would result in lots of pointless extra allocating and freeing.) > I still think there is another leak somewhere because the memory usage still > grows, but much slower than before. I see a small leak in open_for_read_ready(): it should be unreffing the stream in the error case. But that would only cause a leak for unsuccessful read attempts, not successful ones.
(In reply to comment #12) > Created an attachment (id=124381) [edit] > gvfs copying same 4MB .ogg under valgrind WITH peter's patch Again, you MUST set "G_SLICE=always-malloc" in the environment when running valgrind, or there's no way to actually know if objects are being leaked based on the valgrind output.
Created attachment 124382 [details] [review] fix two more leaks this should fix the remaining large leak in the valgrind output, plus the leak-on-error I mentioned above
Created attachment 124383 [details] [review] fixed leak fix oops, ignore previous patch. this should fix it.
(In reply to comment #14) > Again, you MUST set "G_SLICE=always-malloc" in the environment when running > valgrind, or there's no way to actually know if objects are being leaked based > on the valgrind output. In the first line of the logs I attached I used the command: G_SLICE=always-malloc valgrind --show-reachable=yes --leak-check=full /usr/lib/gvfs/gvfsd-http --spawner :1.5 /org/gtk/gvfs/exec_spaw/3 >/dev/null Are you saying this is not sufficient? Do I need to set it using another method?
oops, i clicked "Save Changes", and then while waiting for bugzilla to catch up, I realized I was wrong (the valgrind trace shows g_slice_alloc calls, but gslice is immediately calling malloc), so I hit stop, and reloaded the page, and didn't see my comment, so I figured I'd managed to prevent it from being posted, which is why I didn't start my next comment with "never mind". Your valgrind trace is fine, and the patch I posted should fix the remaining large leak.
Created attachment 124389 [details] gvfs copying a 64MB file 5 times with peters and dan's patches applied (the patch seems to be very effective) Thanks for the clarification Dan, and more importantly thanks for the patch!
Looks good to me!
(In reply to comment #13) > (In reply to comment #9) > > I have attached a patch that disables the accumulation, and now gvfsd-http's > > memory usage does not explode when I fetch large files. > > That memory should all be freed when the message is destroyed. So if disabling > accumulate results in less memory leaked, then that implies that the > SoupMessage must not be getting destroyed... > > (Your patch is still correct though; even if we fixed the leak, leaving > accumulate enabled would result in lots of pointless extra allocating and > freeing.) > I agree the the accumulate buffer should not plug any leaks, but it enables you to copy large files. I am using g_file_copy to download media files (sometimes hundreds of MB in size) and if I don't disable accumulation gvfsd-http needs to use enough ram to hold the entire file. Thanks for the unref patch Dan I'll try it out today.
committed
*** Bug 568462 has been marked as a duplicate of this bug. ***