Bug 561789 – [volume] deadlocks with a controller attached

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 561789 - [volume] deadlocks with a controller attached


Summary:	[volume] deadlocks with a controller attached


Status:	RESOLVED FIXED

Product:	GStreamer
Classification:	Platform
Component:	gst-plugins-base
Version:	git master
Hardware:	Other Linux

Importance:	High blocker
Target Milestone:	0.10.22
Assigned To:	GStreamer Maintainers
QA Contact:	GStreamer Maintainers

URL:
Whiteboard:

Depends on:
Blocks:

Reported:	2008-11-21 10:59 UTC by Jonathan Matthew
Modified:	2008-11-24 12:03 UTC

See Also:
GNOME target:	---
GNOME version:	---

Attachments
horrible example program (392 bytes, text/plain) 2008-11-21 11:00 UTC, Jonathan Matthew		Details
testcase for tests/check/elements/volume.c (2.46 KB, patch) 2008-11-21 14:08 UTC, Jonathan Matthew	committed	Details \| Review

Description Jonathan Matthew 2008-11-21 10:59:11 UTC

Since this change: http://webcvs.freedesktop.org/gstreamer/gst-plugins-base/gst/volume/gstvolume.c?r1=1.101&r2=1.102

attaching a controller to a volume element has resulted in a streaming thread deadlocking.

Comment 1 Jonathan Matthew 2008-11-21 11:00:58 UTC

Created attachment 123164 [details]
horrible example program

Comment 2 Jonathan Matthew 2008-11-21 11:04:18 UTC

And here's a stack trace:

(gdb) i th
  2 Thread 0xb73bdb90 (LWP 19256)  0xb7fdb424 in __kernel_vsyscall ()
  1 Thread 0xb7e1b8c0 (LWP 19254)  0xb7fdb424 in __kernel_vsyscall ()
(gdb) t 2
[Switching to thread 2 (Thread 0xb73bdb90 (LWP 19256))]#0  0xb7fdb424 in __kernel_vsyscall ()
(gdb) bt

+ Trace 210074

#0 __kernel_vsyscall
#1 __lll_lock_wait
from /lib/i686/cmov/libpthread.so.0
#2 _L_lock_89
from /lib/i686/cmov/libpthread.so.0
#3 pthread_mutex_lock
from /lib/i686/cmov/libpthread.so.0
#4 volume_set_property
at gstvolume.c line 770
#5 IA__g_object_set_property
at /tmp/buildd/glib2.0-2.16.6/gobject/gobject.c line 697
#6 gst_controller_sync_values
at gstcontroller.c line 698
#7 gst_object_sync_values
at gsthelper.c line 187
#8 volume_transform_ip
at gstvolume.c line 715
#9 gst_base_transform_handle_buffer
at gstbasetransform.c line 1816
#10 gst_base_transform_chain
at gstbasetransform.c line 1921
#11 gst_pad_chain_unchecked
at gstpad.c line 3890
#12 gst_pad_push
at gstpad.c line 4057
#13 gst_base_src_loop
at gstbasesrc.c line 2275
#14 gst_task_func
at gsttask.c line 192
#15 g_thread_pool_thread_proxy
at /tmp/buildd/glib2.0-2.16.6/glib/gthreadpool.c line 265
#16 g_thread_create_proxy
at /tmp/buildd/glib2.0-2.16.6/glib/gthread.c line 635
#17 start_thread
from /lib/i686/cmov/libpthread.so.0
#18 clone
from /lib/i686/cmov/libc.so.6


What appears to be happening is that the base transform lock is acquired in frame 10 (before calling gst_base_transform_handle_buffer) and frame 4 is trying to acquire it again.

Comment 3 Jonathan Matthew 2008-11-21 14:08:29 UTC

Created attachment 123172 [details] [review]
testcase for tests/check/elements/volume.c

Comment 4 Stefan Sauer (gstreamer, gtkdoc dev) 2008-11-21 21:54:22 UTC

hmm, that is strange. I use dozen controllers in buzztard and have never got bitten by that one.

We could
GST_BASE_TRANSFORM_UNLOCK (this);
gst_object_sync_values (G_OBJECT (this), timestamp);
GST_BASE_TRANSFORM_LOCK (this);

but thats quite a hack.

Comment 5 Wim Taymans 2008-11-24 10:53:34 UTC

I broke this recently. We should not release an unfixed 0.10.22.

Comment 6 Wim Taymans 2008-11-24 12:03:10 UTC

        * gst/volume/gstvolume.c: (volume_choose_func),
        (volume_update_volume), (gst_volume_set_volume),
        (gst_volume_get_volume), (gst_volume_set_mute),
        (gst_volume_class_init), (gst_volume_init),
        (volume_process_double), (volume_process_float),
        (volume_process_int32), (volume_process_int32_clamp),
        (volume_process_int24), (volume_process_int24_clamp),
        (volume_process_int16), (volume_process_int16_clamp),
        (volume_process_int8), (volume_process_int8_clamp), (volume_setup),
        (volume_transform_ip), (volume_set_property),
        (volume_get_property):
        * gst/volume/gstvolume.h:
        Cleanup volume, define and use default values.
        Recalculate new volume and mute setup before processing. Fixes #561789.

        * tests/check/elements/volume.c: (GST_START_TEST), (volume_suite):
        Add controller unit test. Patch by: Jonathan Matthew
        Fix bogus test that messed with basetransform's internal state.