GNOME Bugzilla – Bug 619533
[avimux, matroskamux, flvmux] crash when receiving tags on multiple pads at the same time
Last modified: 2010-05-25 23:20:03 UTC
Not a regression, but would still be nice to fix. Surprised not more people have run into this: Starting program: /home/tpm/gst/releases/gstreamer/tools/.libs/gst-launch-0.10 webmmux name=mux \! fakesink filesrc location=/home/tpm/samples/sintel-trailer.ogv \! decodebin2 name=d d. \! ffmpegcolorspace \! videorate \! vp8enc \! queue \! mux.video_0 d. \! audioconvert \! audiorate \! vorbisenc \! queue \! mux.audio_0 ... *** glibc detected *** /home/tpm/gst/releases/gstreamer/tools/.libs/gst-launch-0.10: double free or corruption (fasttop): 0x00000000008fb5f0 *** ... Program received signal SIGABRT, Aborted.
+ Trace 222071
Thread 140736717728016 (LWP 8804)
Thread 6 (Thread 0x7fffcb5e5910 (LWP 8807))
Thread 5 (Thread 0x7fffcbfff910 (LWP 8806))
Thread 3 (Thread 0x7fffd2112910 (LWP 8804))
Thread 2 (Thread 0x7fffd2913910 (LWP 8803))
Thread 1 (Thread 0x7ffff7fd66f0 (LWP 8802))
Isn't the matroskamux crash fixed by your GstTagSetter changes already? There's another problem (a memory leak) with that code though and locking should be added nonetheless (the pad could be released during tag parsing).
Created attachment 161926 [details] [review] protect tag setter operations with object lock Should be fixed by the tag setter changes, yes. But those are only in git so far... Maybe it's not worth fixing for the release, but if it's the first thing I get when trying to reencode a file to webm/vp8, chances are other people will also run into it.
PS: the tag list copy should probably also go into the critical section then..
Yes, let's get this in and remove it immediately after the release. Even with the GstTagSetter fixes there's a race though: GST_OBJECT_LOCK (avimux); mode = gst_tag_setter_get_tag_merge_mode (setter); gst_tag_setter_merge_tags (setter, list, mode); GST_OBJECT_UNLOCK (avimux); The mode could change between the two gst_tag_setter calls.
> Even with the GstTagSetter fixes there's a race though: > > GST_OBJECT_LOCK (avimux); > mode = gst_tag_setter_get_tag_merge_mode (setter); > gst_tag_setter_merge_tags (setter, list, mode); > GST_OBJECT_UNLOCK (avimux); > > The mode could change between the two gst_tag_setter calls. Well sure, but no one should be changing the mode in state >READY anyway. It seems a rather unlikely corner case, and I'd be more worried then about two threads simultaneously creating the TagData chunk and one thread indirectly invalidating the one the other continues to use. Also, it doesn't really matter if the mode changes just after we read it. It's just potluck which value gets chosen by the streaming thread in this case, there's no defined behaviour. (This is assuming we read the value correctly, which would not necessarily be the case strictly speaking, but again: purely hypothetical unlikely corner case IMHO).
Just committed this now as it is, which is the minimal fix: commit 6a9983cd20c48b96396229b3f94d0254a05ddf48 Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Mon May 24 17:26:42 2010 +0100 avimux, flvmux, matroskamux: don't crash if tags arrive on multiple input pads at the same time This is a temporary fix for the release only. Fixes #619533. We're just trying to protect against concurrency within the plugin here to avoid crashes; the application shouldn't be messing with the tagsetter once the pipeline is running anyway, so let's just ignore that scenario, it will be fixed in a few weeks time as well automatically.