GNOME Bugzilla – Bug 591318
ghostpad : core dump : on_src_target_notify called with an invalid ghostpad
Last modified: 2009-08-18 09:34:17 UTC
My pipeline has a tee with ghosted src request pads. There is a race between caps notification on ghosted pads and ghost pad release. Pushing a buffer makes pad_push notify a caps changed on pad. gst_pad_push -> gst_pad_configure_src -> gst_pad_set_caps -> g_object_notify(pad, "caps") -> on_src_target_notify(pad, unused, ghostpad) Unlike the proxypad which has been reffed in gst_pad_push as the peer pad, the ghost pad has not been reffed and might then be released in another thread. This will lead to the use of an invalid pointer in on_src_target_notify. on_src_target_notify should use the proxy pad instead of the ghost pad, and access the ghost pad through the proxy pad, with mutex to ensure ghost pad is valid. I use a revision from March 2009 (something betwwen 0.10.22 and 23 I think, with some patches), but I did not see any changes on git for this problem.
Additional tests with git revisions show same behaviour. Running under valgrind : ==00:00:05:49.713 28409== Invalid read of size 4 ==00:00:05:49.713 28409== at 0x439AB86: on_src_target_notify (gstghostpad.c:717) ==00:00:05:49.713 28409== by 0x443E1DB: g_cclosure_marshal_VOID__PARAM (gmarshal.c:531) ==00:00:05:49.713 28409== by 0x443113A: g_closure_invoke (gclosure.c:490) ==00:00:05:49.713 28409== by 0x44448B4: signal_emit_unlocked_R (gsignal.c:2440) ==00:00:05:49.713 28409== by 0x4445DED: g_signal_emit_valist (gsignal.c:2199) ==00:00:05:49.713 28409== by 0x4446235: g_signal_emit (gsignal.c:2243) ==00:00:05:49.713 28409== by 0x44356A0: g_object_dispatch_properties_changed (gobject.c:562) ==00:00:05:49.713 28409== by 0x436FAD2: gst_object_dispatch_properties_changed (gstobject.c:509) ==00:00:05:49.713 28409== by 0x4431E9E: g_object_notify_dispatcher (gobject.c:245) ==00:00:05:49.713 28409== by 0x44389DF: g_object_notify (gobjectnotifyqueue.c:138) ==00:00:05:49.713 28409== by 0x43AFB76: gst_pad_set_caps (gstpad.c:2529) ==00:00:05:49.713 28409== by 0x43E63C7: link_fold_func (gstutils.c:2508) ==00:00:05:49.713 28409== by 0x43A33A3: gst_iterator_fold (gstiterator.c:545) ==00:00:05:49.713 28409== by 0x43E664B: gst_pad_proxy_setcaps (gstutils.c:2553) ==00:00:05:49.713 28409== by 0x43AFA30: gst_pad_set_caps (gstpad.c:2515) ==00:00:05:49.713 28409== by 0x43E63C7: link_fold_func (gstutils.c:2508) ==00:00:05:49.713 28409== by 0x43A33A3: gst_iterator_fold (gstiterator.c:545) ==00:00:05:49.714 28409== by 0x43E664B: gst_pad_proxy_setcaps (gstutils.c:2553) ==00:00:05:49.714 28409== by 0x43AFA30: gst_pad_set_caps (gstpad.c:2515) ==00:00:05:49.714 28409== by 0x43E63C7: link_fold_func (gstutils.c:2508) ==00:00:05:49.714 28409== by 0x43A33A3: gst_iterator_fold (gstiterator.c:545) ==00:00:05:49.714 28409== by 0x43E664B: gst_pad_proxy_setcaps (gstutils.c:2553) ==00:00:05:49.714 28409== by 0x43AFA30: gst_pad_set_caps (gstpad.c:2515) ==00:00:05:49.714 28409== by 0x4399460: gst_proxy_pad_do_setcaps (gstghostpad.c:303) ==00:00:05:49.714 28409== by 0x439AC4F: gst_ghost_pad_do_setcaps (gstghostpad.c:736) ==00:00:05:49.714 28409== by 0x43AFA30: gst_pad_set_caps (gstpad.c:2515) ==00:00:05:49.714 28409== by 0x4399460: gst_proxy_pad_do_setcaps (gstghostpad.c:303) ==00:00:05:49.714 28409== by 0x43AFA30: gst_pad_set_caps (gstpad.c:2515) ==00:00:05:49.714 28409== by 0x43AFD40: gst_pad_configure_sink (gstpad.c:2571) ==00:00:05:49.714 28409== by 0x43B3738: gst_pad_chain_data_unchecked (gstpad.c:4013) ==00:00:05:49.714 28409== by 0x43B42FF: gst_pad_push_data (gstpad.c:4260) ==00:00:05:49.714 28409== by 0x43B488B: gst_pad_push (gstpad.c:4364) ==00:00:05:49.714 28409== by 0x73CF451: gst_rtp_pt_demux_chain (gstrtpptdemux.c:389) ==00:00:05:49.714 28409== by 0x43B37D5: gst_pad_chain_data_unchecked (gstpad.c:4031) ==00:00:05:49.714 28409== by 0x43B42FF: gst_pad_push_data (gstpad.c:4260) ==00:00:05:49.714 28409== by 0x43B488B: gst_pad_push (gstpad.c:4364) ==00:00:05:49.714 28409== by 0x73CCA63: gst_rtp_jitter_buffer_loop (gstrtpjitterbuffer.c:1655) ==00:00:05:49.714 28409== by 0x43DA2A8: gst_task_func (gsttask.c:234) ==00:00:05:49.714 28409== by 0x43DB4B0: default_func (gsttaskpool.c:70) ==00:00:05:49.714 28409== by 0x46340C5: g_thread_pool_thread_proxy (gthreadpool.c:265) The ghost pad has been released in a previous thread : ==00:00:05:49.714 28409== Address 0x94efa40 is 16 bytes inside a block of size 232 free'd ==00:00:05:49.714 28409== at 0x402390A: free (vg_replace_malloc.c:323) ==00:00:05:49.714 28409== by 0x4610D35: g_free (gmem.c:190) ==00:00:05:49.714 28409== by 0x444FCA8: g_type_free_instance (gtype.c:1608) ==00:00:05:49.714 28409== by 0x436F170: gst_object_unref (gstobject.c:326) ==00:00:05:49.714 28409== by 0x41BBC1A: RTPBChannelSender::release_audio_record_pad(int) (RTPBChannelSender.cc:4783) ==00:00:05:49.714 28409== by 0x40CE66B: RTPBChannel::waitAudioRTPSourceEnd(RTPBChannelRTPSource*) (RTPBChannel.cc:7762) ==00:00:05:49.714 28409== by 0x40FB182: _audio_rtpsource_wait_end_thread(void*) (RTPBChannel.cc:7715) ==00:00:05:49.714 28409== by 0x42A5557: omni_thread_wrapper (posix.cc:441) ==00:00:05:49.714 28409== by 0x483D32E: start_thread (in /lib/libpthread-2.8.so) ==00:00:05:49.714 28409== by 0x492E20D: clone (in /lib/libc-2.8.so) ==00:00:05:49.733 28409==
Here is a small program that core dumped after one hour and a half. It builds a source bin with audiotestsrc providing buffers each 20 ms linked to tee. tee is always linked to sinkbin1 (queue -> fakesink) via ghost pads. tee is linked/unlinked to sinkbin2 (queue -> fakesink) via ghost pads. I attach the program and the core backtrace.
Created attachment 140631 [details] source code
Created attachment 140632 [details] core backtrace Core was made on a 1 CPU hypertrhreaded host.
Created attachment 140633 [details] Valgrind log Core was also obtained under Valgrind on a Bi-CPU quadcore host. on_src_target_notify uses here an invalid ghostpad. It has been freed in previous unlink.
Created attachment 140634 [details] Valgrind core backtrace
Created attachment 140974 [details] [review] ghostpad-capschange.diff Proposed patch. Without the patch your sample application crashes very fast, with this patch it runs at least for 10 minutes and I guess it will run forever ;)
Created attachment 140975 [details] [review] ghostpad-capschange.diff Actually the ref should only be released after the notify call...
Thanks for the patch, will try it out soon.
commit a0ed1a44a54604a4efae7f926d70e00021db09b6 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Tue Aug 18 11:33:17 2009 +0200 ghostpad: Always get the proxypad's ghostpad via the ghostpad in the src cap Before the signal handler would get the ghostpad passed as second argument but it could've already been unreffed and destroyed. This would then lead to crashes and all that. Now we get the ghostpad from the proxy pad, which we get from the target pad as it's peer. Fixes bug #591318.