GNOME Bugzilla – Bug 787144
Memory leak in gstqtmux.c
Last modified: 2018-11-03 15:21:46 UTC
Created attachment 358954 [details] [review] proposed fix Hello! I have an application which takes video stream from IP camera and streams it as video/mp4 through web-server. The application uses the following pipeline to produce browsers-friendly mp4-stream: appsrc -> h264parse -> qtmux -> appsink The problem is the application leaks GstBuffer-s steadily. AddressSanitizer reports the following 2 stacks with direct leaks: Direct leak of 43792 byte(s) in 161 object(s) allocated from: #0 0x7f97ca5ba73f malloc (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x5473f) #1 0x7f978e613f16 in g_malloc glib-2.42.1/glib/gmem.c:97 #2 0x7f978e64b4ec in g_slice_alloc glib-2.42.1/glib/gslice.c:1007 #3 0x7f978edce8b3 in gst_buffer_new gstreamer-1.11.2/gst/gstbuffer.c:798 #4 0x7f978edcea05 in gst_buffer_new_allocate gstreamer-1.11.2/gst/gstbuffer.c:846 #5 0x7f97705743a8 in gst_h264_parse_wrap_nal gst-plugins-bad-1.11.2/gst/videoparsers/gsth264parse.c:424 #6 0x7f9770577c7b in gst_h264_parse_process_nal gst-plugins-bad-1.11.2/gst/videoparsers/gsth264parse.c:889 #7 0x7f977057a9e3 in gst_h264_parse_handle_frame gst-plugins-bad-1.11.2/gst/videoparsers/gsth264parse.c:1222 #8 0x7f9770863093 in gst_base_parse_handle_buffer gstreamer-1.11.2/libs/gst/base/gstbaseparse.c:2145 #9 0x7f977086ee72 in gst_base_parse_chain gstreamer-1.11.2/libs/gst/base/gstbaseparse.c:3227 #10 0x7f978ee6cd74 in gst_pad_chain_data_unchecked gstreamer-1.11.2/gst/gstpad.c:4205 #11 0x7f978ee6e76d in gst_pad_push_data gstreamer-1.11.2/gst/gstpad.c:4457 #12 0x7f978ee6f73c in gst_pad_push gstreamer-1.11.2/gst/gstpad.c:4576 #13 0x7f97708aed19 in gst_base_src_loop gstreamer-1.11.2/libs/gst/base/gstbasesrc.c:2843 #14 0x7f978eee4ede in gst_task_func gstreamer-1.11.2/gst/gsttask.c:335 #15 0x7f978eee6e4f in default_func gstreamer-1.11.2/gst/gsttaskpool.c:69 #16 0x7f978e667b9d in g_thread_pool_thread_proxy glib-2.42.1/glib/gthreadpool.c:307 #17 0x7f978e666eb6 in g_thread_proxy glib-2.42.1/glib/gthread.c:764 #18 0x7f97c3b05063 in start_thread /build/glibc-6V9RKT/glibc-2.19/nptl/pthread_create.c:309 (discriminator 2) Direct leak of 8976 byte(s) in 33 object(s) allocated from: #0 0x7f97ca5ba73f malloc (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x5473f) #1 0x7f978e613f16 in g_malloc glib-2.42.1/glib/gmem.c:97 #2 0x7f978e64b4ec in g_slice_alloc glib-2.42.1/glib/gslice.c:1007 #3 0x7f978edce8b3 in gst_buffer_new gstreamer-1.11.2/gst/gstbuffer.c:798 #4 0x7f97708d109f in gst_byte_writer_reset_and_get_buffer gstreamer-1.11.2/libs/gst/base/gstbytewriter.c:244 #5 0x7f9770581b0a in gst_h264_parse_handle_sps_pps_nals gst-plugins-bad-1.11.2/gst/videoparsers/gsth264parse.c:2306 #6 0x7f9770582ccf in gst_h264_parse_pre_push_frame gst-plugins-bad-1.11.2/gst/videoparsers/gsth264parse.c:2417 #7 0x7f9770866ead in gst_base_parse_push_frame gstreamer-1.11.2/libs/gst/base/gstbaseparse.c:2455 #8 0x7f9770865783 in gst_base_parse_handle_and_push_frame gstreamer-1.11.2/libs/gst/base/gstbaseparse.c:2337 #9 0x7f9770868e5a in gst_base_parse_finish_frame gstreamer-1.11.2/libs/gst/base/gstbaseparse.c:2678 #10 0x7f977057ac44 in gst_h264_parse_handle_frame gst-plugins-bad-1.11.2/gst/videoparsers/gsth264parse.c:1248 #11 0x7f9770863093 in gst_base_parse_handle_buffer gstreamer-1.11.2/libs/gst/base/gstbaseparse.c:2145 #12 0x7f977086ee72 in gst_base_parse_chain gstreamer-1.11.2/libs/gst/base/gstbaseparse.c:3227 #13 0x7f978ee6cd74 in gst_pad_chain_data_unchecked gstreamer-1.11.2/gst/gstpad.c:4205 #14 0x7f978ee6e76d in gst_pad_push_data gstreamer-1.11.2/gst/gstpad.c:4457 #15 0x7f978ee6f73c in gst_pad_push gstreamer-1.11.2/gst/gstpad.c:4576 #16 0x7f97708aed19 in gst_base_src_loop gstreamer-1.11.2/libs/gst/base/gstbasesrc.c:2843 #17 0x7f978eee4ede in gst_task_func gstreamer-1.11.2/gst/gsttask.c:335 #18 0x7f978eee6e4f in default_func gstreamer-1.11.2/gst/gsttaskpool.c:69 #19 0x7f978e667b9d in g_thread_pool_thread_proxy glib-2.42.1/glib/gthreadpool.c:307 #20 0x7f978e666eb6 in g_thread_proxy glib-2.42.1/glib/gthread.c:764 #21 0x7f97c3b05063 in start_thread /build/glibc-6V9RKT/glibc-2.19/nptl/pthread_create.c:309 (discriminator 2) With help of additional logs in gst-plugins-good-1.11.2/gst/isomp4/gstqtmux.c it was noticed that the buffers leak when the pipeline is about to stop (on client disconnect) and there were some buffers in GstQTPad::fragment_buffers. Unfortunately, I was not able to find the easy way to reproduce such conditions with simple gst-launch command line use-case (tried filesrc and rtspsrc). The proposed fix is attached.
Thanks for the patch! Please submit the patch against latest GIT master or 1.12 (yours does not apply anymore), and provide it as a "git format-patch" style patch. See https://gstreamer.freedesktop.org/documentation/contribute/index.html#how-to-submit-patches The change itself looks correct.
Hello Sebastian! Thanks for your reply. I've just figured out that I forgot about qtmux attributes my application sets: fragment-duration=100 faststart=1 So now I'm able to reproduce the leak with simple gst-launch command: gst-launch-1.0 rtspsrc location=rtsp://url ! queue ! rtph264depay ! h264parse config-interval=-1 ! qtmux fragment-duration=100 faststart=1 ! filesink location=out.mp4 Moreover, I've found that the leak does not reproduce when filesrc is used, i.e.: gst-launch-1.0 filesrc location=/home/builder/10.mp4 ! qtdemux ! h264parse config-interval=-1 ! qtmux fragment-duration=100 faststart=1 ! filesink location=out.mp4 It turned out that the difference was in EOS. filesrc sends EOS automatically on EOF. When pipeline with rtspsrc is interrupted with SIGINT, EOS is not sent by default and the leak is observed. So the workaround is to force EOS generation on shut down: gst-launch-1.0 --eos-on-shutdown rtspsrc location=rtsp://url ! queue ! rtph264depay ! h264parse config-interval=-1 ! qtmux fragment-duration=100 faststart=1 ! filesink location=out.mp4 I've checked the originally proposed fix with gst-launch command. Unfortunately, gst-launch crashes with heap-use-after-free (on gst_buffer_unref called on already freed object). It looks like there is a data race around GstQTPad::fragment_buffers between gst_qt_mux_pad_reset invoked during pipeline shutdown chain and gst_qt_mux_pad_fragment_add_buffer still using it in different thread. I believe the data race can be eliminated by making access to GstQTPad::fragment_buffers atomic either with proper synchronization or with some lock-free technique.
*** Bug 787225 has been marked as a duplicate of this bug. ***
If those are indeed different thread than it means that gst_qt_mux_collected is called while GST_STATE_CHANGE_PAUSED_TO_READY, which, if happens, seams like a bigger problem than the original leak. Or perhaps the state is changed from thread that has gst_qt_mux_pad_fragment_add_buffer in while the muxer sends out buffer? In which case the solution would perhaps be to copy the array content to stack before iterating over it in gst_qt_mux_pad_fragment_add_buffer and clear pad->fragment_buffers before sending out the first buffer.
Please, ignore my previous statement regarding the data race I thought I'd found. Today I had time to investigate the problem more carefully. Unfortunately, I haven't kept original crash report so now I'm not sure whether I indeed saw gst_buffer_unref in the crash stack (I was in a little bit of hurry reporting the patch can lead to crash). All stacks I've collected today were triggered by gst_debug_print_object@gstreamer-1.11.2/gst/gstinfo.c:885 (i.e. by tracing routines, not the gst_buffer_unref call) and the guilty object turned out to be qtpad (not the leaking fragment buffer as I thought previously).
+ Trace 237925
... and nothing data race alike in different threads. To be sure I've replaced GST_TRACE_OBJECT with GST_TRACE and got rid of qtpad reference and gst-launch has ceased to crash. As for the crashing on accessing qtpad object I did not investigate further whether it is expected behavior at this point of a pipeline destruction. I suppose it might be so. CONCLUSION: the patch I originally proposed should be rejected, without tracing line it must be safe enough - this is exactly what Matej posted at https://bugzilla.gnome.org/attachment.cgi?id=359046
Review of attachment 358954 [details] [review]: can lead to crash, but without tracing line it must be safe enough
The qtpad reference there must be valid, otherwise there is a bug elsewhere. This needs further debugging.
Created attachment 359353 [details] ASan report for crash on shutdown with EOS with patch applied Got crash on gst_buffer_unref with the tracing-free patch applied :( To reproduce you need just to shutdown with EOS, i.e. to run the command that previously allowed to workaround the leak: gst-launch-1.0 --eos-on-shutdown rtspsrc location=rtsp://url ! queue ! rtph264depay ! h264parse config-interval=-1 ! qtmux fragment-duration=100 faststart=1 ! filesink location=out.mp4
Ok, so this is the actual buffer from h264parse that is unreffed twice. Once in qtmux (causing the problem), and previously the last time in basesink (where it was destroyed). The reference counting of those buffers has to be double checked. Does qtmux actually keep a reference to them around (it should), or pass them all downstream / unref?
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/399.