Bug 186678 – [GStreamer] Some media stream tests are crashing

• Component: Media
• Status: RESOLVED FIXED

[Michael Catanzaro]

Layout test fast/mediastream/change-tracks-media-stream-being-played.html often crashes deep in GStreamer:

Thread 1 (Thread 0x7f6c768e23c0 (LWP 24453)):
#0  gst_stream_get_stream_id () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gststreams.c:265
#1  0x00007f6c1d58e550 in get_output_for_slot () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gst-plugins-base-1.14.1/gst/playback/gstdecodebin3.c:1444
#2  0x00007f6c1d594854 in idle_reconfigure () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gst-plugins-base-1.14.1/gst/playback/gstdecodebin3.c:2207
#3  0x00007f6c7f53e5c2 in gst_pad_add_probe () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstpad.c:1505
#4  0x00007f6c1d58f288 in handle_stream_switch () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gst-plugins-base-1.14.1/gst/playback/gstdecodebin3.c:2544
#5  0x00007f6c1d58f818 in ghost_pad_event_probe () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gst-plugins-base-1.14.1/gst/playback/gstdecodebin3.c:2618
#6  0x00007f6c7f539f8d in probe_hook_marshal () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstpad.c:3562
#7  0x00007f6c7e4c3844 in g_hook_list_marshal () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/ghook.c:672
#8  0x00007f6c7f5385ea in do_probe_callbacks () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstpad.c:3721
#9  0x00007f6c7f53b34c in gst_pad_push_event_unchecked () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstpad.c:5368
#10 0x00007f6c7f545871 in gst_pad_push_event () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstpad.c:5531
#11 0x00007f6c7f545c6e in event_forward_func () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstpad.c:3054
#12 0x00007f6c7f54168e in gst_pad_forward () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstpad.c:3008
#13 0x00007f6c7f5417c3 in gst_pad_event_default () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstpad.c:3105
#14 0x00007f6c7f53b067 in gst_pad_send_event_unchecked () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstpad.c:5738
#15 0x00007f6c7f53b502 in gst_pad_push_event_unchecked () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstpad.c:5394
#16 0x00007f6c7f545871 in gst_pad_push_event () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstpad.c:5531
#17 0x00007f6c7f545c6e in event_forward_func () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstpad.c:3054
#18 0x00007f6c7f54168e in gst_pad_forward () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstpad.c:3008
#19 0x00007f6c7f5417c3 in gst_pad_event_default () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstpad.c:3105
#20 0x00007f6c7f53b067 in gst_pad_send_event_unchecked () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstpad.c:5738
#21 0x00007f6c7f545d79 in gst_pad_send_event () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstpad.c:5908
#22 0x00007f6c7f501de8 in gst_bin_send_event () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstbin.c:3188
#23 0x00007f6c7f522bf9 in gst_element_send_event () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstelement.c:1857
#24 0x00007f6c1d5bb93e in gst_play_bin3_send_event () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gst-plugins-base-1.14.1/gst/playback/gstplaybin3.c:2293
#25 0x00007f6c7f522bf9 in gst_element_send_event () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/gstreamer-1.14.1/gst/gstelement.c:1857
#26 0x00007f6c866e593e in _ZN7WebCore27MediaPlayerPrivateGStreamer11enableTrackENS_25TrackPrivateBaseGStreamer9TrackTypeEj () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#27 0x00007f6c866f8345 in _ZN7WebCore26VideoTrackPrivateGStreamerC2EN3WTF7WeakPtrINS_27MediaPlayerPrivateGStreamerEEEiNS1_7GRefPtrI10_GstStreamEE () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#28 0x00007f6c866eb639 in _ZN7WebCore27MediaPlayerPrivateGStreamer12updateTracksEv () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#29 0x00007f6c866e3d7a in _ZN3WTF8FunctionIFvvEE15CallableWrapperIZN7WebCore18MainThreadNotifierINS4_31MediaPlayerPrivateGStreamerBase22MainThreadNotificationEE6notifyIZNS4_27MediaPlayerPrivateGStreamer17handleSyncMessageEP11_GstMessageEUlvE_EEvS7_OT_EUlvE_E4callEv () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#30 0x00007f6c82ea2488 in _ZN3WTF7RunLoop11performWorkEv () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#31 0x00007f6c82ed8669 in _ZZN3WTF7RunLoopC4EvENUlPvE_4_FUNES1_ () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#32 0x00007f6c7e4d281a in g_main_dispatch () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gmain.c:3148
#33 g_main_context_dispatch () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gmain.c:3813
#34 0x00007f6c7e4d2ba8 in g_main_context_iterate () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gmain.c:3886
#35 0x00007f6c7e4d2ec2 in g_main_loop_run () at /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gmain.c:4082
#36 0x00007f6c82ed9060 in _ZN3WTF7RunLoop3runEv () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#37 0x00007f6c8538f7a2 in _ZN6WebKit16ChildProcessMainINS_10WebProcessENS_14WebProcessMainEEEiiPPc () from /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#38 0x00007f6c7c5112b1 in __libc_start_main (main=0x55cac516fcb0 <main>, argc=3, argv=0x7ffcf3108778, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffcf3108768) at ../csu/libc-start.c:291
#39 0x000055cac516fd3a in _start ()

[Michael Catanzaro]

Also fast/mediastream/MediaStream-video-element-track-stop.html

[Thibault Saunier]

Created attachment 342940 [details]
Patch.

Note that I was not able to reproduce those crashes running the tests locally
or on our buildbox but I had a very similare segfault running MiniBrowser in
some rare cases and this patch seems/should to fix it. I think it is conceptually
correct/better anyway.

I am investigating the root cause of that issue, but the decodebin3 code is still
moving quite fast and not being able to reproduce properly myself is annoying.

The approach taken here is to make sure to not do "stupid things" on our end so
that we avoid hitting the race in decodebin3, still the bug is still there.

[EWS Watchlist]

Attachment 342940 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:17:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
ERROR: Source/WebCore/ChangeLog:18:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Thibault Saunier]

Created attachment 342941 [details]
Patch.

Fix style in ChangeLog

[Thibault Saunier]

Comment on attachment 342941 [details]
Patch.

I am digging further and might have found something else, do not review yet.

[Thibault Saunier]

Created attachment 342970 [details]
Patch.

Also avoid spurious SELECT_STREAM for GstStream marked with GST_STREAM_FLAG_SELECT
as this flags makes it so decodebin3 selects them by default.

[Michael Catanzaro]

Comment on attachment 342970 [details]
Patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=342970&action=review

> Source/WebCore/ChangeLog:16
> +        Should fix following flaks:

flakes

[EWS Watchlist]

Comment on attachment 342970 [details]
Patch.

Attachment 342970 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/8245539

New failing tests:
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-video.html
http/tests/security/contentSecurityPolicy/video-redirect-allowed.html

[EWS Watchlist]

Created attachment 343044 [details]
Archive of layout-test-results from ews204 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Thibault Saunier]

(In reply to Build Bot from comment #9)
> Created attachment 343044 [details]
> Archive of layout-test-results from ews204 for win-future
> 
> The attached test failures were seen while running run-webkit-tests on the
> win-ews.
> Bot: ews204  Port: win-future  Platform:
> CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

Seems totally unrelated to me.

[Philippe Normand]

Comment on attachment 342970 [details]
Patch.

Looks good! Would be nice to avoid the goto though, probably by using a Vector for the streams list and only converting it to a GList when needed. Would it be possible?

[Thibault Saunier]

Created attachment 343328 [details]
Patch.

Add a missing check to avoid calling gst_stream_collection_get () on a NULL collection
when using playbin2. Removed the goto by using a Vector<String> and convert that to a
GList only when required.

[Philippe Normand]

Comment on attachment 343328 [details]
Patch.

Thanks :)

[WebKit Commit Bot]

Comment on attachment 343328 [details]
Patch.

Clearing flags on attachment: 343328

Committed r233081: <https://trac.webkit.org/changeset/233081>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/41370477>

[Philippe Normand]

Comment on attachment 343328 [details]
Patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=343328&action=review

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:755
> +    GstStream* stream = nullptr;
> +
> +    if (!m_isLegacyPlaybin) {
> +        gst_stream_collection_get_stream(m_streamCollection.get(), index);
> +        if (!stream) {
> +            GST_WARNING_OBJECT(pipeline(), "No stream to select at index %u", index);
> +            return;
> +        }

Was this actually tested? :)
I've just noticed now that stream is never set :D

[Thibault Saunier]

Comment on attachment 343328 [details]
Patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=343328&action=review

>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:755
>> +        }
> 
> Was this actually tested? :)
> I've just noticed now that stream is never set :D

Rooops, sorry about that, stupid refactoring before proposing again.