Bug 127452

Summary: [GStreamer] Lockup when playing Icecast radio
Product: WebKit Reporter: Brendan Long <b.long>
Component: MediaAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, commit-queue, gustavo, menard, mrobinson, pnormand
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
URL: http://dir.xiph.org/index.php
Attachments:
Description Flags
Patch
none
Patch none

Brendan Long
Reported 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.
Attachments
Patch (1.92 KB, patch)
2014-01-23 11:08 PST, Brendan Long 		no flags
DetailsFormatted DiffDiff
Patch (2.02 KB, patch)
2014-01-24 10:01 PST, Brendan Long 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Brendan Long
Comment 1 2014-01-23 11:08:28 PST
Created attachment 222005 [details] Patch
Brendan Long
Comment 2 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.
Brendan Long
Comment 3 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.
Gustavo Noronha (kov)
Comment 4 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.
Brendan Long
Comment 5 2014-01-24 10:01:43 PST
Created attachment 222116 [details] Patch Like this?
Philippe Normand
Comment 6 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?
Philippe Normand
Comment 7 2014-01-26 20:06:31 PST
Comment on attachment 222116 [details] Patch Looks good, thanks!
WebKit Commit Bot
Comment 8 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>
WebKit Commit Bot
Comment 9 2014-01-27 08:53:02 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.