| Summary: | [WinCairo] Crash when closing window while video is loading | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Mathieu Lafon <arcoun> | ||||
| Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | achristensen, andersca, bfulgham, commit-queue, peavo | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| Attachments: |
|
||||||
|
Description
Mathieu Lafon
2015-03-26 03:05:54 PDT
What svn revision was this seen on? Can it be reproduced after r181805? It looks like MediaPlayerPrivateMediaFoundation::createSession should also be protected by a mutex kind of like https://bugs.webkit.org/show_bug.cgi?id=142578 Could you upload a patch? I'm having a little trouble reproducing this one, which OS are you on? I'm testing WinLauncher 32-bit on Win7. After additional testing, here are some complementary informations: - The crash is triggered by ~MediaPlayerPrivateMediaFoundation when releasing m_topology (IMFTopology) - The crash occurs on a Win7 VM with mf.dll 12.0.7601.17514 - The crash does not occur on an other Win7 VM with mf.dll 12.0.7601.18741 but m_topology is not set in MediaPlayerPrivateMediaFoundation destructor in that case (video is still loading). I'm going to do some more testing to better understand if m_toppology should be set but this seems to be related to MediaFoundation version. Created attachment 249760 [details]
Patch
(In reply to comment #5) > After additional testing, here are some complementary informations: > > - The crash is triggered by ~MediaPlayerPrivateMediaFoundation when > releasing m_topology (IMFTopology) > > - The crash occurs on a Win7 VM with mf.dll 12.0.7601.17514 > > - The crash does not occur on an other Win7 VM with mf.dll 12.0.7601.18741 > but m_topology is not set in MediaPlayerPrivateMediaFoundation destructor in > that case (video is still loading). > > I'm going to do some more testing to better understand if m_toppology should > be set but this seems to be related to MediaFoundation version. Could you try the uploaded patch? I'm not sure this will fix your problem, but I believe it fixes a potential crash if a call on the main thread made by a worker thread is invoked after the media player is destroyed. This looks good to me. Anders, what do you think? Are there other places where callOnMainThread(std::function<void()>) could cause similar crashes in WebKit? Is this how things like this are fixed? Comment on attachment 249760 [details]
Patch
r=me
Comment on attachment 249760 [details] Patch Clearing flags on attachment: 249760 Committed r182160: <http://trac.webkit.org/changeset/182160> All reviewed patches have been landed. Closing bug. This may have broken accessibility/aria-hidden-hides-all-elements.html on Windows. https://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=accessibility%2Faria-hidden-hides-all-elements.html This patch does not resolve the problem I am experiencing. I still get a crash. Regarding my comment on the VM with mf.dll 12.0.7601.18741, m_topology is also set but there is no crash when ~MediaPlayerPrivateMediaFoundation release it. So this bug is surely related to a bug in MediaFoundation which has been fixed in the latest versions. I let you decide if the status should be modified (INVALID?). Not sure this is related to MediaFoundation, mf.dll or a Windows update. After installing every updates available in Windows update, I still have the same crash... (In reply to comment #12) > This may have broken accessibility/aria-hidden-hides-all-elements.html on > Windows. > > https://webkit-test-results.appspot.com/dashboards/flakiness_dashboard. > html#showAllRuns=true&tests=accessibility%2Faria-hidden-hides-all-elements. > html There is no way this caused that test failure. The AppleWin port doesn't even compile this code. (In reply to comment #14) > Not sure this is related to MediaFoundation, mf.dll or a Windows update. After installing every updates available in Windows update, I still have the same crash... It's possible we're still not handling MediaFoundation correctly. Try to reproduce that crash, file another bug, debug it, and upload a patch :) (In reply to comment #14) > Not sure this is related to MediaFoundation, mf.dll or a Windows update. > After installing every updates available in Windows update, I still have the > same crash... Could you try to apply the following patch? It seems to be recommended to release all MF objects before MFShutdown is called. Index: MediaPlayerPrivateMediaFoundation.cpp =================================================================== --- MediaPlayerPrivateMediaFoundation.cpp (revisjon 182189) +++ MediaPlayerPrivateMediaFoundation.cpp (arbeidskopi) @@ -293,6 +293,13 @@ bool MediaPlayerPrivateMediaFoundation::endSession() { + // Release all MF objects before MFShutdown is called. + m_sourceResolver = nullptr; + m_mediaSource = nullptr; + m_topology = nullptr; + m_sourcePD = nullptr; + m_videoDisplay = nullptr; + if (m_mediaSession) { m_mediaSession->Shutdown(); m_mediaSession = nullptr; (In reply to comment #15) > It's possible we're still not handling MediaFoundation correctly. Try to > reproduce that crash, file another bug, debug it, and upload a patch :) Why file another bug? I still easily reproduce this same bug, with the same stack trace and using the same condition. Nothing has been resolved here so the status should not be RESOLVED/FIXED. (In reply to comment #16) > Could you try to apply the following patch? > It seems to be recommended to release all MF objects before MFShutdown is > called. No, still got a crash... Now during m_mediaSession->Shutdown but this might be because this is where the problematic object is completely released. -- I only reproduce this in a VMWare VM, this might be related to the VMWare video driver or tools. Anyone has the same configuration? This seems to be directly due to the VMWare SVGA driver. - I've got the same crash (in winsat.exe, exactly same stack trace) when testing the Windows Experience Index - If I remove the VMWare SVGA driver, no crash in winsat.exe or WebKit. - If I reinstall the VMWare SVGA driver, winsat.exe and WebKit both crash. I think it can be closed as INVALID and related to 3rdparty drivers. This investigation led us to fix a problem that needed to be fixed, but I believe your specific problem is indeed a problem with 3rd party drivers. If you find that there is indeed code in WebKit that does something wrong with MediaFoundation that causes a crash, please file another bug. Thanks! |