| Summary: | [GStreamer][STABLE] Crash in WebCore::MediaPlayer::createResourceLoader | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> |
| Component: | Media | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED WORKSFORME | ||
| Severity: | Normal | CC: | aboya, bugs-noreply, calvaris, mcatanzaro, pnormand |
| Priority: | P2 | ||
| Version: | WebKit Nightly Build | ||
| Hardware: | PC | ||
| OS: | Linux | ||
|
Description
Michael Catanzaro
2020-05-07 08:11:07 PDT
The presence of `notifyAsyncCompletion` calls my attention. That argument (along with async start) has been removed in the WebKitWebSrc rework, which is most likely the element behind this bug and has been largely modified since. Hmm, is it safe to backport that work to 2.28? (Probably not?) BTW, maybe time for a 2.29 release? It's now a really long time for Tech Preview to not be following trunk. (In reply to Michael Catanzaro from comment #2) > Hmm, is it safe to backport that work to 2.28? (Probably not?) > As it provides stability fixes and no new features, I would backport it, if it's not too much trouble. (In reply to Michael Catanzaro from comment #2) > Hmm, is it safe to backport that work to 2.28? (Probably not?) It's still a bit risky since it's a recent land with substantial changes, but it fixes issues. Probably safest to release 2.29.1 first, so we can start testing it. Then, once we have had a couple weeks to do basic testing, then maybe consider backporting. But if you don't want to investigate a stable branch crash, then we need to backport no matter what, because this crash is 100%. This is the 3rd bug report about createResourceLoader()... Are we sure there's no duplicate? See bug 204035 and bug 204161. *** Bug 204161 has been marked as a duplicate of this bug. *** *** Bug 204035 has been marked as a duplicate of this bug. *** I guess what Sorry, it seems pressing tab by mistake shifts focus to Save Changes, and then pressing Space on the Save Changes button submits the comment. <_< I'm not sure why I failed to find those bugs before reporting this. I remember being surprised when I searched Bugzilla and couldn't find any dups.... Anyway, what's different now is that, when I reported the first two bugs, the crash was random and non-reproducible. Now it's no longer possible to load the affected webpage without hitting the crash. Also note Phil had an interesting comment at: https://bugs.webkit.org/show_bug.cgi?id=204161#c1 (In reply to Michael Catanzaro from comment #10) > Also note Phil had an interesting comment at: > https://bugs.webkit.org/show_bug.cgi?id=204161#c1 This is Phil's comment: > This might be a regression introduced by r247215 ... > Capturing the src pointer in the lambda of webKitWebSrcMakeRequest() doesn't look right; protector should be used instead. After the WebKitWebSrc rework, webKitWebSrcStart() (the function modified in that commit) has been almost emptied, all lambdas use protectors, and care has taken to cancel pending callbacks when the pipeline is flushed or torn down. These bugs are no longer relevant with the WebKitWebSrc rework. As of today, the page is, incredibly, no longer crashing in Tech Preview. Who knows what has changed. (In reply to Alicia Boya García from comment #11) > After the WebKitWebSrc rework, webKitWebSrcStart() (the function modified in > that commit) has been almost emptied, all lambdas use protectors, and care > has taken to cancel pending callbacks when the pipeline is flushed or torn > down. These bugs are no longer relevant with the WebKitWebSrc rework. Alicia, maybe you should make a decision as to whether your commits should be backported (and if so, which ones), or whether this requires investigation on the current stable branch? (In reply to Michael Catanzaro from comment #12) > As of today, the page is, incredibly, no longer crashing in Tech Preview. > Who knows what has changed. > > (In reply to Alicia Boya García from comment #11) > > After the WebKitWebSrc rework, webKitWebSrcStart() (the function modified in > > that commit) has been almost emptied, all lambdas use protectors, and care > > has taken to cancel pending callbacks when the pipeline is flushed or torn > > down. These bugs are no longer relevant with the WebKitWebSrc rework. > > Alicia, maybe you should make a decision as to whether your commits should > be backported (and if so, which ones), or whether this requires > investigation on the current stable branch? My decision is to backport. In fact, it's already in the backport list: https://trac.webkit.org/wiki/WebKitGTK/2.28.x (In reply to Michael Catanzaro from comment #5) > Probably safest to release 2.29.1 first, so we can start testing it. Then, > once we have had a couple weeks to do basic testing, then maybe consider > backporting. Thanks for this release, Carlos. We can begin testing Alicia's changes soon... let's give in a couple weeks to see if users report regressions before backporting. So far, I've noticed some video playback hangs on YouTube that seem to be newly-introduced, which may or may not be related. But seek is definitely working much better than it was before (and the seeking bugs were more severe IMO). (In reply to Michael Catanzaro from comment #15) > So far, I've noticed some video playback hangs on YouTube that seem to be > newly-introduced, which may or may not be related. But seek is definitely > working much better than it was before (and the seeking bugs were more > severe IMO). YouTube uses MSE so therefore it doesn't use WebKitWebSrc or createResourceLoader(). Any perceived improvement or regression is unrelated to the WebKitWebSrc threading rework. Is this bug reproduciable? I could not. It was 100% reproducible at the time, but it looks like it stopped crashing about a year ago, see comment #5. Whatever. |