After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 619533 - [avimux, matroskamux, flvmux] crash when receiving tags on multiple pads at the same time
[avimux, matroskamux, flvmux] crash when receiving tags on multiple pads at t...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal blocker
: 0.10.23
Assigned To: Tim-Philipp Müller
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-05-24 16:22 UTC by Tim-Philipp Müller
Modified: 2010-05-25 23:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
protect tag setter operations with object lock (3.86 KB, patch)
2010-05-25 11:04 UTC, Tim-Philipp Müller
committed Details | Review

Description Tim-Philipp Müller 2010-05-24 16:22:50 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.

Thread 140736717728016 (LWP 8804)

  • #0 *__GI_raise
    at ../nptl/sysdeps/unix/sysv/linux/raise.c line 64
  • #1 *__GI_abort
    at abort.c line 88
  • #2 __libc_message
    at ../sysdeps/unix/sysv/linux/libc_fatal.c line 173
  • #3 malloc_printerr
  • #4 _int_realloc
    at malloc.c line 5297
  • #5 *__GI___libc_realloc
    at malloc.c line 3821
  • #6 IA__g_realloc
    at /tmp/buildd/glib2.0-2.24.1/glib/gmem.c line 171
  • #7 g_array_maybe_expand
    at /tmp/buildd/glib2.0-2.24.1/glib/garray.c line 687
  • #8 IA__g_array_append_vals
    at /tmp/buildd/glib2.0-2.24.1/glib/garray.c line 354
  • #9 gst_structure_id_set_value
    at gststructure.c line 443
  • #10 gst_tag_list_add_value_internal
    at gsttaglist.c line 741
  • #11 gst_tag_list_copy_foreach
    at gsttaglist.c line 757
  • #12 gst_structure_foreach
    at gststructure.c line 985
  • #13 gst_tag_list_insert
    at gsttaglist.c line 785
  • #14 gst_matroska_mux_handle_sink_event
    at matroska-mux.c line 628
  • #15 gst_pad_send_event
    at gstpad.c line 5052
  • #16 gst_pad_push_event
    at gstpad.c line 4908
  • #17 gst_queue_push_one
    at gstqueue.c line 1129
  • #18 gst_queue_loop
    at gstqueue.c line 1185
  • #19 gst_task_func
    at gsttask.c line 271
  • #20 g_thread_pool_thread_proxy
    at /tmp/buildd/glib2.0-2.24.1/glib/gthreadpool.c line 315
  • #21 g_thread_create_proxy
    at /tmp/buildd/glib2.0-2.24.1/glib/gthread.c line 1893
  • #22 start_thread
    at pthread_create.c line 300
  • #23 clone
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 112
  • #24 ??

Thread 6 (Thread 0x7fffcb5e5910 (LWP 8807))

  • #0 __lll_lock_wait_private
    at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S line 97
  • #1 _L_lock_9464
    from /lib/libc.so.6
  • #2 *__GI___libc_free
    at malloc.c line 3736
  • #3 gst_buffer_finalize
    at gstbuffer.c line 199
  • #4 gst_mini_object_free
    at gstminiobject.c line 336
  • #5 gst_mini_object_unref
    at gstminiobject.c line 371
  • #6 gst_buffer_unref
    at /home/tpm/gst/releases/gstreamer/gst/gstbuffer.h line 363
  • #7 vorbis_dec_chain_forward
    at gstvorbisdec.c line 1150
  • #8 vorbis_dec_chain
    at gstvorbisdec.c line 1177
  • #9 gst_pad_chain_data_unchecked
    at gstpad.c line 4131
  • #10 gst_pad_push_data
    at gstpad.c line 4360
  • #11 gst_single_queue_push_one
    at gstmultiqueue.c line 919
  • #12 gst_multi_queue_loop
    at gstmultiqueue.c line 1101
  • #13 gst_task_func
    at gsttask.c line 271
  • #14 g_thread_pool_thread_proxy
    at /tmp/buildd/glib2.0-2.24.1/glib/gthreadpool.c line 315
  • #15 g_thread_create_proxy
    at /tmp/buildd/glib2.0-2.24.1/glib/gthread.c line 1893
  • #16 start_thread
    at pthread_create.c line 300
  • #17 clone
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 112
  • #18 ??

Thread 5 (Thread 0x7fffcbfff910 (LWP 8806))

  • #0 __lll_lock_wait_private
    at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S line 97
  • #1 _L_lock_9464
    from /lib/libc.so.6
  • #2 *__GI___libc_free
    at malloc.c line 3736
  • #3 gst_buffer_finalize
    at gstbuffer.c line 199
  • #4 gst_mini_object_free
    at gstminiobject.c line 336
  • #5 gst_mini_object_unref
    at gstminiobject.c line 371
  • #6 gst_buffer_unref
    at /home/tpm/gst/releases/gstreamer/gst/gstbuffer.h line 363
  • #7 theora_dec_chain_forward
    at gsttheoradec.c line 1385
  • #8 theora_dec_chain
    at gsttheoradec.c line 1411
  • #9 gst_pad_chain_data_unchecked
    at gstpad.c line 4131
  • #10 gst_pad_push_data
    at gstpad.c line 4360
  • #11 gst_single_queue_push_one
    at gstmultiqueue.c line 919
  • #12 gst_multi_queue_loop
    at gstmultiqueue.c line 1101
  • #13 gst_task_func
    at gsttask.c line 271
  • #14 g_thread_pool_thread_proxy
    at /tmp/buildd/glib2.0-2.24.1/glib/gthreadpool.c line 315
  • #15 g_thread_create_proxy
    at /tmp/buildd/glib2.0-2.24.1/glib/gthread.c line 1893
  • #16 start_thread
    at pthread_create.c line 300
  • #17 clone
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 112
  • #18 ??

Thread 3 (Thread 0x7fffd2112910 (LWP 8804))

  • #0 *__GI_raise
    at ../nptl/sysdeps/unix/sysv/linux/raise.c line 64
  • #1 *__GI_abort
    at abort.c line 88
  • #2 __libc_message
    at ../sysdeps/unix/sysv/linux/libc_fatal.c line 173
  • #3 malloc_printerr
  • #4 _int_realloc
    at malloc.c line 5297
  • #5 *__GI___libc_realloc
    at malloc.c line 3821
  • #6 IA__g_realloc
    at /tmp/buildd/glib2.0-2.24.1/glib/gmem.c line 171
  • #7 g_array_maybe_expand
    at /tmp/buildd/glib2.0-2.24.1/glib/garray.c line 687
  • #8 IA__g_array_append_vals
    at /tmp/buildd/glib2.0-2.24.1/glib/garray.c line 354
  • #9 gst_structure_id_set_value
    at gststructure.c line 443
  • #10 gst_tag_list_add_value_internal
    at gsttaglist.c line 741
  • #11 gst_tag_list_copy_foreach
    at gsttaglist.c line 757
  • #12 gst_structure_foreach
    at gststructure.c line 985
  • #13 gst_tag_list_insert
    at gsttaglist.c line 785
  • #14 gst_matroska_mux_handle_sink_event
    at matroska-mux.c line 628
  • #15 gst_pad_send_event
    at gstpad.c line 5052
  • #16 gst_pad_push_event
    at gstpad.c line 4908
  • #17 gst_queue_push_one
    at gstqueue.c line 1129
  • #18 gst_queue_loop
    at gstqueue.c line 1185
  • #19 gst_task_func
    at gsttask.c line 271
  • #20 g_thread_pool_thread_proxy
    at /tmp/buildd/glib2.0-2.24.1/glib/gthreadpool.c line 315
  • #21 g_thread_create_proxy
    at /tmp/buildd/glib2.0-2.24.1/glib/gthread.c line 1893
  • #22 start_thread
    at pthread_create.c line 300
  • #23 clone
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 112
  • #24 ??

Thread 2 (Thread 0x7fffd2913910 (LWP 8803))

  • #0 __lll_lock_wait_private
    at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S line 97
  • #1 _L_lock_11277
    from /lib/libc.so.6
  • #2 *__GI___libc_realloc
    at malloc.c line 3813
  • #3 IA__g_realloc
    at /tmp/buildd/glib2.0-2.24.1/glib/gmem.c line 171
  • #4 g_array_maybe_expand
    at /tmp/buildd/glib2.0-2.24.1/glib/garray.c line 687
  • #5 IA__g_array_append_vals
    at /tmp/buildd/glib2.0-2.24.1/glib/garray.c line 354
  • #6 gst_structure_id_set_value
    at gststructure.c line 443
  • #7 gst_tag_list_add_value_internal
    at gsttaglist.c line 741
  • #8 gst_tag_list_copy_foreach
    at gsttaglist.c line 757
  • #9 gst_structure_foreach
    at gststructure.c line 985
  • #10 gst_tag_list_insert
    at gsttaglist.c line 785
  • #11 gst_matroska_mux_handle_sink_event
    at matroska-mux.c line 628
  • #12 gst_pad_send_event
    at gstpad.c line 5052
  • #13 gst_pad_push_event
    at gstpad.c line 4908
  • #14 gst_queue_push_one
    at gstqueue.c line 1129
  • #15 gst_queue_loop
    at gstqueue.c line 1185
  • #16 gst_task_func
    at gsttask.c line 271
  • #17 g_thread_pool_thread_proxy
    at /tmp/buildd/glib2.0-2.24.1/glib/gthreadpool.c line 315
  • #18 g_thread_create_proxy
    at /tmp/buildd/glib2.0-2.24.1/glib/gthread.c line 1893
  • #19 start_thread
    at pthread_create.c line 300
  • #20 clone
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 112
  • #21 ??

Thread 1 (Thread 0x7ffff7fd66f0 (LWP 8802))

  • #0 __lll_lock_wait_private
    at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S line 97
  • #1 _L_lock_9464
    from /lib/libc.so.6
  • #2 *__GI___libc_free
    at malloc.c line 3736
  • #3 g_source_destroy_internal
    at /tmp/buildd/glib2.0-2.24.1/glib/gmain.c line 856
  • #4 IA__g_source_remove
    at /tmp/buildd/glib2.0-2.24.1/glib/gmain.c line 1527
  • #5 gst_bus_poll
    at gstbus.c line 1060
  • #6 event_loop
    at gst-launch.c line 405
  • #7 main
    at gst-launch.c line 827

Comment 1 Sebastian Dröge (slomo) 2010-05-25 09:50:46 UTC
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).
Comment 2 Tim-Philipp Müller 2010-05-25 11:04:38 UTC
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.
Comment 3 Tim-Philipp Müller 2010-05-25 11:07:41 UTC
PS: the tag list copy should probably also go into the critical section then..
Comment 4 Sebastian Dröge (slomo) 2010-05-25 16:16:57 UTC
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.
Comment 5 Tim-Philipp Müller 2010-05-25 16:37:11 UTC
> 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).
Comment 6 Tim-Philipp Müller 2010-05-25 23:19:38 UTC
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.