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 673413 - tsdemux causes random segfaults on start of playback
tsdemux causes random segfaults on start of playback
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal critical
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-04-03 09:05 UTC by Julian Scheel
Modified: 2012-05-24 07:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix for double free. (1.15 KB, patch)
2012-04-04 08:35 UTC, Julian Scheel
none Details | Review

Description Julian Scheel 2012-04-03 09:05:31 UTC
Using gst-plugins-bad from git 0.10 branch one gets segementation faults with mpegts input, when a pipeline is stopped and started again. The crash leads to many different backtraces, but this one seems to be most interesting:

--

Program received signal SIGSEGV, Segmentation fault.

Thread 140737235744512 (LWP 31258)

  • #0 g_slice_alloc
    from /usr/lib/libglib-2.0.so.0
  • #1 g_list_prepend
    from /usr/lib/libglib-2.0.so.0
  • #2 gst_ts_demux_queue_data
    at ../../../gst/mpegtsdemux/tsdemux.c line 1382
  • #3 gst_ts_demux_handle_packet
    at ../../../gst/mpegtsdemux/tsdemux.c line 1591
  • #4 gst_ts_demux_push
    at ../../../gst/mpegtsdemux/tsdemux.c line 1623
  • #5 mpegts_base_push
    at ../../../gst/mpegtsdemux/mpegtsbase.c line 1362
  • #6 mpegts_base_chain
    at ../../../gst/mpegtsdemux/mpegtsbase.c line 1413
  • #7 mpegts_base_loop
    at ../../../gst/mpegtsdemux/mpegtsbase.c line 1557
  • #8 gst_task_func
    at gsttask.c line 328
  • #9 ??
    from /usr/lib/libglib-2.0.so.0
  • #10 ??
    from /usr/lib/libglib-2.0.so.0
  • #11 start_thread
    from /lib/libpthread.so.0
  • #12 clone
    from /lib/libc.so.6

To easily reproduce this I used gst-auto-launch tool from https://github.com/tigrux/gst-auto-launch. Calling it like this:

gst-auto-launch playbin2 flags=0x160 videosink="glessink" uri=file:///home/julian/full.ts 0:play 10:stop 11:play 20:stop 21:play

The crash does not occur when testing the same with the ts file remuxed into mp4 container, so it seems to be tsdemux specific.
Comment 1 Julian Scheel 2012-04-03 09:27:49 UTC
Valgrind memcheck shows this result:

==31306== Memcheck, a memory error detector
==31306== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==31306== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==31306== Command: gst-auto-launch playbin2 flags=0x160 videosink=fakesink audiosink=fakesink uri=file:///home/julian/full.ts 0:play 10:stop 11:play 20:stop 21:play
==31306== 
GStreamer has detected that it is running inside valgrind.
It might now take different code paths to ease debugging.
Of course, this may also lead to different bugs.
Passing to PLAYING
==31306== Thread 5:
==31306== Conditional jump or move depends on uninitialised value(s)
==31306==    at 0xC24DB84: pa_sample_spec_valid (in /usr/lib/libpulse.so.0.13.5)
==31306==    by 0xC252908: ??? (in /usr/lib/libpulse.so.0.13.5)
==31306==    by 0xC252FEE: pa_stream_connect_playback (in /usr/lib/libpulse.so.0.13.5)
==31306==    by 0xBDDAD69: ??? (in /usr/lib/gstreamer-0.10/libgstpulse.so)
==31306==    by 0x50E319B: gst_pad_accept_caps (gstpad.c:2628)
==31306==    by 0x80656F5: sink_accepts_caps (gstplaybin2.c:3236)
==31306==    by 0x8065F7E: autoplug_select_cb (gstplaybin2.c:3404)
==31306==    by 0x8B7948F: gst_play_marshal_ENUM__OBJECT_BOXED_OBJECT (gstplay-marshal.c:292)
==31306==    by 0x587A0E3: g_closure_invoke (in /usr/lib/libgobject-2.0.so.0.3000.2)
==31306==    by 0x588C069: ??? (in /usr/lib/libgobject-2.0.so.0.3000.2)
==31306==    by 0x58954C2: g_signal_emit_valist (in /usr/lib/libgobject-2.0.so.0.3000.2)
==31306==    by 0x5895891: g_signal_emit (in /usr/lib/libgobject-2.0.so.0.3000.2)
==31306== 
==31306== Thread 6:
==31306== Syscall param socketcall.sendto(msg) points to uninitialised byte(s)
==31306==    at 0x672A88C: send (in /lib/libpthread-2.15.so)
==31306==    by 0xC694DE4: pa_write (in /usr/lib/libpulsecommon-1.1.so)
==31306==    by 0xC69D38E: pa_iochannel_write (in /usr/lib/libpulsecommon-1.1.so)
==31306==    by 0xC6AEA78: ??? (in /usr/lib/libpulsecommon-1.1.so)
==31306==    by 0xC24AC8D: pa_mainloop_dispatch (in /usr/lib/libpulse.so.0.13.5)
==31306==    by 0xC24B054: pa_mainloop_iterate (in /usr/lib/libpulse.so.0.13.5)
==31306==    by 0xC24B0FF: pa_mainloop_run (in /usr/lib/libpulse.so.0.13.5)
==31306==    by 0xC25956E: ??? (in /usr/lib/libpulse.so.0.13.5)
==31306==    by 0xC6BC8B7: ??? (in /usr/lib/libpulsecommon-1.1.so)
==31306==    by 0x6723DA9: start_thread (in /lib/libpthread-2.15.so)
==31306==  Address 0xa6d213c is 12 bytes inside a block of size 281 alloc'd
==31306==    at 0x4C29A22: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==31306==    by 0xC25F196: pa_xrealloc (in /usr/lib/libpulse.so.0.13.5)
==31306==    by 0xC6B5E0C: ??? (in /usr/lib/libpulsecommon-1.1.so)
==31306==    by 0xC6B6949: pa_tagstruct_put_cvolume (in /usr/lib/libpulsecommon-1.1.so)
==31306==    by 0xC2528FB: ??? (in /usr/lib/libpulse.so.0.13.5)
==31306==    by 0xC252FEE: pa_stream_connect_playback (in /usr/lib/libpulse.so.0.13.5)
==31306==    by 0xBDDAD69: ??? (in /usr/lib/gstreamer-0.10/libgstpulse.so)
==31306==    by 0x50E319B: gst_pad_accept_caps (gstpad.c:2628)
==31306==    by 0x80656F5: sink_accepts_caps (gstplaybin2.c:3236)
==31306==    by 0x8065F7E: autoplug_select_cb (gstplaybin2.c:3404)
==31306==    by 0x8B7948F: gst_play_marshal_ENUM__OBJECT_BOXED_OBJECT (gstplay-marshal.c:292)
==31306==    by 0x587A0E3: g_closure_invoke (in /usr/lib/libgobject-2.0.so.0.3000.2)
==31306== 
0:00:09.479196724 31306      0xa7190d0 ERROR                 ffmpeg :0:: frame sync error
==31306== Thread 10:
==31306== Conditional jump or move depends on uninitialised value(s)
==31306==    at 0xC24DB84: pa_sample_spec_valid (in /usr/lib/libpulse.so.0.13.5)
==31306==    by 0xC252908: ??? (in /usr/lib/libpulse.so.0.13.5)
==31306==    by 0xC252FEE: pa_stream_connect_playback (in /usr/lib/libpulse.so.0.13.5)
==31306==    by 0xBDE0E38: ??? (in /usr/lib/gstreamer-0.10/libgstpulse.so)
==31306==    by 0xBFFE801: gst_ring_buffer_acquire (gstringbuffer.c:817)
==31306==    by 0xC00F042: gst_base_audio_sink_setcaps (gstbaseaudiosink.c:921)
==31306==    by 0x4E4C323: gst_base_sink_pad_setcaps (gstbasesink.c:635)
==31306==    by 0x50E352E: gst_pad_set_caps (gstpad.c:2730)
==31306==    by 0x50D19A0: gst_proxy_pad_setcaps_default (gstghostpad.c:490)
==31306==    by 0x50E352E: gst_pad_set_caps (gstpad.c:2730)
==31306==    by 0x50E4F5F: gst_pad_push_data (gstpad.c:4247)
==31306==    by 0x50E8AD5: gst_pad_push (gstpad.c:4730)
==31306== 
Passing to READY
Passing to PLAYING
==31306== Thread 6:
==31306== Invalid read of size 8
==31306==    at 0x6486EE5: g_slice_alloc (in /usr/lib/libglib-2.0.so.0.3000.2)
==31306==    by 0x646629D: g_list_prepend (in /usr/lib/libglib-2.0.so.0.3000.2)
==31306==    by 0x9749F51: gst_ts_demux_push (tsdemux.c:1382)
==31306==    by 0x973B86E: mpegts_base_chain (mpegtsbase.c:1362)
==31306==    by 0x973BD63: mpegts_base_loop (mpegtsbase.c:1557)
==31306==    by 0x5110123: gst_task_func (gsttask.c:328)
==31306==    by 0x64936A7: ??? (in /usr/lib/libglib-2.0.so.0.3000.2)
==31306==    by 0x6491185: ??? (in /usr/lib/libglib-2.0.so.0.3000.2)
==31306==    by 0x6723DA9: start_thread (in /lib/libpthread-2.15.so)
==31306==  Address 0x4000000002 is not stack'd, malloc'd or (recently) free'd
==31306== 
==31306== 
==31306== Process terminating with default action of signal 11 (SIGSEGV)
==31306==  Access not within mapped region at address 0x4000000002
==31306==    at 0x6486EE5: g_slice_alloc (in /usr/lib/libglib-2.0.so.0.3000.2)
==31306==    by 0x646629D: g_list_prepend (in /usr/lib/libglib-2.0.so.0.3000.2)
==31306==    by 0x9749F51: gst_ts_demux_push (tsdemux.c:1382)
==31306==    by 0x973B86E: mpegts_base_chain (mpegtsbase.c:1362)
==31306==    by 0x973BD63: mpegts_base_loop (mpegtsbase.c:1557)
==31306==    by 0x5110123: gst_task_func (gsttask.c:328)
==31306==    by 0x64936A7: ??? (in /usr/lib/libglib-2.0.so.0.3000.2)
==31306==    by 0x6491185: ??? (in /usr/lib/libglib-2.0.so.0.3000.2)
==31306==    by 0x6723DA9: start_thread (in /lib/libpthread-2.15.so)
==31306==  If you believe this happened as a result of a stack
==31306==  overflow in your program's main thread (unlikely but
==31306==  possible), you can try to increase the size of the
==31306==  main thread stack using the --main-stacksize= flag.
==31306==  The main thread stack size used in this run was 8388608.
==31306== 
==31306== HEAP SUMMARY:
==31306==     in use at exit: 3,379,114 bytes in 16,943 blocks
==31306==   total heap usage: 85,096 allocs, 68,153 frees, 24,966,278 bytes allocated
==31306== 
==31306== LEAK SUMMARY:
==31306==    definitely lost: 16,444 bytes in 12 blocks
==31306==    indirectly lost: 0 bytes in 0 blocks
==31306==      possibly lost: 1,500,192 bytes in 4,943 blocks
==31306==    still reachable: 1,862,478 bytes in 11,988 blocks
==31306==         suppressed: 0 bytes in 0 blocks
==31306== Rerun with --leak-check=full to see details of leaked memory
==31306== 
==31306== For counts of detected and suppressed errors, rerun with: -v
==31306== Use --track-origins=yes to see where uninitialised values come from
==31306== ERROR SUMMARY: 11 errors from 4 contexts (suppressed: 3 from 3)
Comment 2 Julian Scheel 2012-04-04 08:35:46 UTC
Created attachment 211263 [details] [review]
Fix for double free.

The attached patch fixed the reported crashes.
Comment 3 Julian Scheel 2012-04-04 08:37:44 UTC
I just see that commit 3b525d1147fd1a632a5dfa07b0ef02155eac585f also addresses this issue, so my patch may be omitted. Although I am unsure if the solution in that commit might not introduce a possible memleak when state is != PENDING_PACKET_BUFFER...
Comment 4 Edward Hervey 2012-05-22 15:58:20 UTC
Does that commit fix the issue for you ? Can you reproduce the issue ?
Comment 5 Julian Scheel 2012-05-23 14:44:56 UTC
I can not reproduce the issue with current git anymore. So I assume this commit fixed it.
But is it really safe to set the pointer to NULL in beach-area?
Couldn't this lead to a memory-leak in the case of

G_UNLIKELY (stream->state != PENDING_PACKET_BUFFER))

there it goes to beach as well, but without processing and cleaning the currentit. This is why I cleaned the pointer explicitly right after unref.
Comment 6 Edward Hervey 2012-05-24 07:20:25 UTC
currentit is gone now. PENDING_PACKET_BUFFER is the only state where data is actually allocated.