Bug 143088 – [WinCairo] Crash when closing window while video is loading

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Mathieu Lafon]

A crash can be easily reproduced when closing a window or leaving a page while the video is still loading (not yet playing).

Test Url can be http://www.quirksmode.org/html5/tests/video.html

Reproduced with WebKit 601.1.24 which include recent related crash fixes:

- Bug 142578 - [WinCairo] Crash when leaving page while video is playing.
- Bug 141248 - [WinCairo] Crash when media player is destroyed.

Also reproduced using WinLauncher, exception analysis (WinDbg) below. Seems to be a double-free.


*******************************************************************************
*                                                                             *
*                        Exception Analysis                                   *
*                                                                             *
*******************************************************************************

FAULTING_IP: 
ntdll!RtlReportCriticalFailure+29
77043845 cc              int     3

EXCEPTION_RECORD:  ffffffff -- (.exr 0xffffffffffffffff)
ExceptionAddress: 77043845 (ntdll!RtlReportCriticalFailure+0x00000029)
   ExceptionCode: 80000003 (Break instruction exception)
  ExceptionFlags: 00000000
NumberParameters: 3
   Parameter[0]: 00000000
   Parameter[1]: 84a30030
   Parameter[2]: 0012ecc9

CONTEXT:  00000000 -- (.cxr 0x0;r)
eax=00000000 ebx=00000000 ecx=76fa179f edx=0012ecc9 esi=04080000 edi=04086bb8
eip=77043845 esp=0012ef1c ebp=0012ef94 iopl=0         nv up ei pl nz na po nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000202
ntdll!RtlReportCriticalFailure+0x29:
77043845 cc              int     3

FAULTING_THREAD:  00000e04

PROCESS_NAME:  WinLauncher.exe

OVERLAPPED_MODULE: Address regions for 'rgb9rast' and 'vm3dum.dll' overlap

ERROR_CODE: (NTSTATUS) 0x80000003 - {EXCEPTION}  Breakpoint  A breakpoint has been reached.

EXCEPTION_CODE: (HRESULT) 0x80000003 (2147483651) - One or more arguments are invalid

EXCEPTION_PARAMETER1:  00000000

EXCEPTION_PARAMETER2:  84a30030

EXCEPTION_PARAMETER3:  0012ecc9

NTGLOBALFLAG:  0

APPLICATION_VERIFIER_FLAGS:  0

APP:  winlauncher.exe

ANALYSIS_VERSION: 6.3.9600.17298 (debuggers(dbg).141024-1500) x86fre

LAST_CONTROL_TRANSFER:  from 770447a3 to 77043845

BUGCHECK_STR:  APPLICATION_FAULT_ACTIONABLE_HEAP_CORRUPTION_heap_failure_block_not_busy_DOUBLE_FREE

PRIMARY_PROBLEM_CLASS:  ACTIONABLE_HEAP_CORRUPTION_heap_failure_block_not_busy

DEFAULT_BUCKET_ID:  ACTIONABLE_HEAP_CORRUPTION_heap_failure_block_not_busy

STACK_TEXT:  
7705ce10 76ffddff ntdll!RtlFreeHeap+0x64
7705ce14 75a8c3d4 kernel32!HeapFree+0x14
7705ce18 62d24c1a d3d9!MemFree+0x1b
7705ce1c 62d39e7e d3d9!CEnum::Release+0xe6
7705ce20 6817314d evr!CMonitorArray9::TerminateDisplaySystem+0x1c
7705ce24 68173691 evr!CMFVideoPresenter::~CMFVideoPresenter+0x8f
7705ce28 68172fda evr!CUnknown::NonDelegatingRelease+0x23
7705ce2c 68162d3e evr!CBaseAllocator::Release+0x11
7705ce30 76c14977 oleaut32!VariantClear+0xb9
7705ce34 76aeb8fa ole32!PropVariantClearWorker+0x72
7705ce38 76af3d88 ole32!PropVariantClear+0xf
7705ce3c 624d54e5 mf!MFCreateMP3MediaSink+0x397a
7705ce40 624d8d4f mf!MFCreateTopology+0x254
7705ce44 624d73b2 mf!MFCreateTopologyNode+0x1a15
7705ce48 624d88e6 mf!MFCreateTopologyNode+0x2f49
7705ce4c 624d5228 mf!MFCreateMP3MediaSink+0x36bd
7705ce50 01db690b webkit!WebCore::MediaPlayerPrivateMediaFoundation::~MediaPlayerPrivateMediaFoundation+0x7b
7705ce54 01c427f6 webkit!WebCore::MediaPlayer::~MediaPlayer+0x86
7705ce58 0164c3d5 webkit!WebCore::HTMLMediaElement::clearMediaPlayer+0x25
7705ce5c 0164c275 webkit!WebCore::HTMLMediaElement::userCancelledLoad+0x25
7705ce60 015d7671 webkit!WebCore::Document::prepareForDestruction+0xa1
7705ce64 01a8b276 webkit!WebCore::Frame::createView+0x36
7705ce68 014c80d2 webkit!WebFrameLoaderClient::transitionToCommittedForNewPage+0xb2
7705ce6c 01571bcc webkit!WebCore::FrameLoader::transitionToCommitted+0x1ac
7705ce70 01570295 webkit!WebCore::FrameLoader::commitProvisionalLoad+0x165
7705ce74 01568c9c webkit!WebCore::DocumentLoader::commitLoad+0x3c
7705ce78 01569a6c webkit!WebCore::DocumentLoader::dataReceived+0x7c
7705ce7c 01bad690 webkit!WebCore::CachedRawResource::notifyClientsDataWasReceived+0x40
7705ce80 01bacdfd webkit!WebCore::CachedRawResource::addDataBuffer+0x8d
7705ce84 01b9a639 webkit!WebCore::SubresourceLoader::didReceiveDataOrBuffer+0x89
7705ce88 01b9a210 webkit!WebCore::SubresourceLoader::didReceiveData+0x20
7705ce8c 01565147 webkit!WebCore::ResourceLoader::didReceiveData+0x17


STACK_COMMAND:  .ecxr ; kb ; dps 7705ce10 ; kb

FOLLOWUP_IP: 
EVR!CMonitorArray9::TerminateDisplaySystem+1c
6817314d 832600          and     dword ptr [esi],0

SYMBOL_STACK_INDEX:  4

SYMBOL_NAME:  evr!CMonitorArray9::TerminateDisplaySystem+1c

FOLLOWUP_NAME:  MachineOwner

MODULE_NAME: EVR

IMAGE_NAME:  EVR.dll

DEBUG_FLR_IMAGE_TIMESTAMP:  4ce7b86c

FAILURE_BUCKET_ID:  ACTIONABLE_HEAP_CORRUPTION_heap_failure_block_not_busy_80000003_EVR.dll!CMonitorArray9::TerminateDisplaySystem

BUCKET_ID:  APPLICATION_FAULT_ACTIONABLE_HEAP_CORRUPTION_heap_failure_block_not_busy_DOUBLE_FREE_evr!CMonitorArray9::TerminateDisplaySystem+1c

ANALYSIS_SOURCE:  UM

FAILURE_ID_HASH_STRING:  um:actionable_heap_corruption_heap_failure_block_not_busy_80000003_evr.dll!cmonitorarray9::terminatedisplaysystem

FAILURE_ID_HASH:  {5042ab70-5cbd-dae5-520a-04daa1b8d317}

Followup: MachineOwner
---------

[Alex Christensen]

What svn revision was this seen on?  Can it be reproduced after r181805?

[Mathieu Lafon]

I am using the Safari-601.1.24 tag (r181960) which include the r181805 fix.

[Alex Christensen]

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.

[Mathieu Lafon]

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.

[Alex Christensen]

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?

[Mark Lam]

Comment on attachment 249760 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 249760 [details]
Patch

Clearing flags on attachment: 249760

Committed r182160: <http://trac.webkit.org/changeset/182160>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

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

[Mathieu Lafon]

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?).

[Mathieu Lafon]

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...

[Alex Christensen]

(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;

[Mathieu Lafon]

(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?

[Mathieu Lafon]

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.

[Alex Christensen]

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!