Bug 127452 - [GStreamer] Lockup when playing Icecast radio
Summary: [GStreamer] Lockup when playing Icecast radio
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL: http://dir.xiph.org/index.php
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-22 16:06 PST by Brendan Long
Modified: 2014-01-27 08:53 PST (History)
6 users (show)

See Also:


Attachments
Patch (1.92 KB, patch)
2014-01-23 11:08 PST, Brendan Long
no flags Details | Formatted Diff | Diff
Patch (2.02 KB, patch)
2014-01-24 10:01 PST, Brendan Long
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brendan Long 2014-01-22 16:06:14 PST
1. Go to this URL: http://dir.xiph.org/index.php
2. Click any of the play buttons on the page.

On my machine it locks up immediately. It could be my particular version of GStreamer or something though (currently using git master from a couple weeks ago). Does this work for anyone?

I'll try to get a backtrace tomorrow.
Comment 1 Brendan Long 2014-01-23 11:08:28 PST
Created attachment 222005 [details]
Patch
Comment 2 Brendan Long 2014-01-23 11:12:20 PST
Looking at the backtrace, the problem was that in StreamingClient::handleResponseReceived(), we'd lock the WebKitWebSrc, then notify on several properties. One of the other threads tried to access the WebKitWebSrc, and then we get a deadlock. I'm pretty sure we're not supposed to notify while holding locks, so I just freeze the notifications while we're locked.

I'm really not that familiar with GLib though, so I'm not sure if this is the preferred way, or if we should do something different, like queuing up the notifications in a Vector<const char*>. This seems to be what freeze/thaw is meant for though.
Comment 3 Brendan Long 2014-01-23 11:14:21 PST
Oh, and I'd like to write a test for this, but it requires streaming media via Icecast (or something else which triggers a notify). I figured it was better to fix the bug than wait until I can figure out how (and find time to) write a test.
Comment 4 Gustavo Noronha (kov) 2014-01-24 05:59:25 PST
Comment on attachment 222005 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        No new tests, just fixing a lockup.

I think it would be best if you describe here what you commented on the bug about the test requiring streaming the media in a way that makes it emit notifications, which makes it complex to have right away.
Comment 5 Brendan Long 2014-01-24 10:01:43 PST
Created attachment 222116 [details]
Patch

Like this?
Comment 6 Philippe Normand 2014-01-26 20:03:39 PST
Some of the http/tests/media tests use CGI scripts to mock media "streaming". Have you looked at those?
Comment 7 Philippe Normand 2014-01-26 20:06:31 PST
Comment on attachment 222116 [details]
Patch

Looks good, thanks!
Comment 8 WebKit Commit Bot 2014-01-27 08:53:00 PST
Comment on attachment 222116 [details]
Patch

Clearing flags on attachment: 222116

Committed r162842: <http://trac.webkit.org/changeset/162842>
Comment 9 WebKit Commit Bot 2014-01-27 08:53:02 PST
All reviewed patches have been landed.  Closing bug.