Bug 683782 – Segfault in pulsesink.c:2077 gst_pulsesink_pad_acceptcaps()

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 683782 - Segfault in pulsesink.c:2077 gst_pulsesink_pad_acceptcaps()


Summary:	Segfault in pulsesink.c:2077 gst_pulsesink_pad_acceptcaps()


Status:	RESOLVED FIXED

Product:	GStreamer
Classification:	Platform
Component:	gst-plugins-good
Version:	0.10.31
Hardware:	Other Linux

Importance:	Normal normal
Target Milestone:	1.0.2
Assigned To:	GStreamer Maintainers
QA Contact:	GStreamer Maintainers

URL:
Whiteboard:

Depends on:
Blocks:

Reported:	2012-09-11 08:26 UTC by Joakim
Modified:	2012-10-19 14:15 UTC

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

Attachments
pulsesink: in accept_caps() check if ring buffer is NULL before de-referencing (2.06 KB, patch) 2012-10-18 10:35 UTC, Tim-Philipp Müller	committed	Details \| Review

Description Joakim 2012-09-11 08:26:53 UTC

When repeatedly setting up and removing a pipeline which plays an mp3 file using pulsesink I sometimes get a segfault. 

I'm running Ubuntu 12.04 and have the following version of gstreamer0.10-pulseaudio, 0.10.31-1ubuntu1.

Backtrace:

+ Trace 230838

#0 __kernel_vsyscall
#1 raise
from /lib/i386-linux-gnu/libc.so.6
#2 abort
from /lib/i386-linux-gnu/libc.so.6
#3 PManager::messageHandler(QtMsgType, char const*)
from ../lib/liblamprey.so.1
#4 qt_message_output
at global/qglobal.cpp line 2203
#5 qt_message
at global/qglobal.cpp line 2322
#6 qFatal
at global/qglobal.cpp line 2505
#7 QTest::FatalSignalHandler::signal
at qtestcase.cpp line 1729
#8 <signal handler called>
#9 gst_pulsesink_pad_acceptcaps
at pulsesink.c line 2077
#10 gst_pad_accept_caps
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#11 gst_proxy_pad_acceptcaps_default
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#12 gst_pad_accept_caps
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#13 ??
from /usr/lib/i386-linux-gnu/libgstbase-0.10.so.0
#14 ??
from /usr/lib/i386-linux-gnu/libgstbase-0.10.so.0
#15 ??
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#16 ??
from /usr/lib/i386-linux-gnu/libgstbase-0.10.so.0
#17 ??
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#18 gst_proxy_pad_bufferalloc_default
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#19 ??
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#20 ??
from /usr/lib/i386-linux-gnu/gstreamer-0.10/libgstcoreelements.so
#21 ??
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#22 gst_proxy_pad_bufferalloc_default
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#23 ??
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#24 ??
from /usr/lib/i386-linux-gnu/gstreamer-0.10/libgstplaybin.so
#25 ??
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#26 ??
from /usr/lib/i386-linux-gnu/gstreamer-0.10/libgstcoreelements.so
#27 ??
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#28 gst_proxy_pad_bufferalloc_default
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#29 ??
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#30 ??
from /usr/lib/i386-linux-gnu/gstreamer-0.10/libgstcoreelements.so
#31 ??
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#32 gst_proxy_pad_bufferalloc_default
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#33 ??
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#34 gst_proxy_pad_bufferalloc_default
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#35 ??
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#36 ??
from /usr/lib/i386-linux-gnu/gstreamer-0.10/libgstflump3dec.so
#37 ??
from /usr/lib/i386-linux-gnu/gstreamer-0.10/libgstflump3dec.so
#38 gst_pad_push
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#39 gst_base_parse_push_frame
from /usr/lib/i386-linux-gnu/libgstbase-0.10.so.0
#40 ??
from /usr/lib/i386-linux-gnu/libgstbase-0.10.so.0
#41 ??
from /usr/lib/i386-linux-gnu/libgstbase-0.10.so.0
#42 ??
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#43 ??
from /usr/lib/i386-linux-gnu/libgstreamer-0.10.so.0
#44 ??
from /lib/i386-linux-gnu/libglib-2.0.so.0
#45 ??
from /lib/i386-linux-gnu/libglib-2.0.so.0
#46 ??
from /usr/lib/nvidia-current/libGL.so.1

Comment 1 Tim-Philipp Müller 2012-09-24 13:37:38 UTC

Could you provide a simple test program, preferably in C, that reproduces this?

Comment 2 Tim-Philipp Müller 2012-10-18 10:35:12 UTC

Created attachment 226722 [details] [review]
pulsesink: in accept_caps() check if ring buffer is NULL before de-referencing

Line 2077 from 0.10.31 was:

  /* Either template caps didn't match, or we're still in NULL state */
  if (!ret || !pbuf->context)
    goto done;

So a crash here could only be caused by pbuf being NULL.

  GstPulseRingBuffer *pbuf = GST_PULSERING_BUFFER_CAST (GST_BASE_AUDIO_SINK (psink)->ringbuffer);

Not sure why/when this would happen though, but it might be possible during shutdown I guess.

Code looks similar in 1.0. Attached patch might help.

Comment 3 Joakim 2012-10-18 14:19:39 UTC

(In reply to comment #2)
 
> Code looks similar in 1.0. Attached patch might help.

Yes, that did the trick, thanks!

Comment 4 Tim-Philipp Müller 2012-10-19 14:15:21 UTC

Thanks! Pushed a slightly different version of the patch, with additional locking for the pbuf->context access. Would be great if you could re-test that as well.

http://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?id=d2fdc26c38acc243ba7912f4db17f63fe0d54632

commit d2fdc26c38acc243ba7912f4db17f63fe0d54632
Author: Tim-Philipp Müller <tim.muller@collabora.co.uk>
Date:   Thu Oct 18 11:32:10 2012 +0100

    pulsesink: in accept_caps() check if ring buffer is NULL before de-referencing
    
    And sprinkle some thread-safety (take object lock for
    accessing ring buffer, and pa main loop lock for the
    context).
    
    https://bugzilla.gnome.org/show_bug.cgi?id=683782