1723868 - Crash in [@ mozilla::FunctionRef<T>::FunctionRef<T>::<T>::__invoke] (msvp9dec_store.dll unloaded, Process = gpu or rdd)

Status: RESOLVED FIXED

(Core :: mozglue, defect)

Description

[Toshihito Kikuchi [:toshi]]

This has the same signature as bug 1722566, but the root cause is different. This crash happened in the call to GetPdbInfo() in SharedLibraryInfo::GetInfoForSelf because the target module was unloaded. The unloaded module is always msvp9dec_store.dll.

Here are examples:

• Crash in GPU:

  • 011aeec1-c09d-424f-bcbb-1d4080210802
  • 338a635f-82a3-45e5-9595-5c81f0210721

• Crash in RDD:

  • 09ec2400-b367-42a0-abed-709520210712
  • d7cdd423-cc1c-4dd5-929b-d13920210728

This problem is the same as bug 1702086, which was fixed by making sure we "lock" a target module by incrementing its refcount or loading it as data. Upon further investigation, it turned out that msvp9dec_store.dll could be unloaded even though we locked the module.

First, msvp9dec_store.dll is loaded into RDD or GPU via CanCreateMFTDecoder with CLSID_WebmMfVpxDec. We delegate a task to create an instance into the COM MTA thread where we create an instance of MFTDecoder and release it.

0d 00000008`1a97da30 00007ffa`1d958263     KERNELBASE!LoadLibraryExW+0x162
0e 00000008`1a97daa0 00007ffa`1d957f3f     mfplat!CMFInprocDllManager::LoadDllInternal+0x77
0f 00000008`1a97db50 00007ffa`1d957c72     mfplat!CMFInprocDllManager::LoadDll+0x73
10 00000008`1a97dbc0 00007ffa`28261d08     mfplat!CMFWinrtInprocActivate::ActivateObject+0x432
11 00000008`1a97dc50 00007ffa`37abb264     MSVP9DEC!CMSVPxDecoderClassFactory::CreateInstance+0x268
...
1f 00000008`1a97f510 00007ff9`c268f8e3     combase!CoCreateInstance+0x10c
20 (Inline Function) --------`--------     xul!mozilla::MFTDecoder::Create+0x37
21 (Inline Function) --------`--------     xul!mozilla::CanCreateMFTDecoder::<unnamed-tag>::operator()+0x87
22 00000008`1a97f5b0 00007ff9`c170ffdc     xul!mozilla::detail::RunnableFunction<`lambda at /builds/worker/checkouts/gecko/dom/media/platforms/wmf/WMFDecoderModule.cpp:97:29'>::Run+0x93
23 (Inline Function) --------`--------     xul!mozilla::mscom::EnsureMTA::SyncDispatchToPersistentThread::<unnamed-tag>::operator()+0x14
24 00000008`1a97f600 00007ff9`c0867ece     xul!mozilla::detail::RunnableFunction<`lambda at /builds/worker/checkouts/gecko/ipc/mscom/EnsureMTA.cpp:233:22'>::Run+0x1c
25 00000008`1a97f630 00007ff9`c086681a     xul!nsThread::ProcessNextEvent+0x13de

If the instance of MFTDecoder is destroyed at the end of the task, it posts a task to unload msvp9dec_store.dll to the work queue like this.

00 000000c2`21fbf0c8 00007ffa`1d95771e     mfplat!MFScheduleWorkItem<<lambda_7e7b75702c9f908394152b2b7b5faf2a> >
01 000000c2`21fbf0d0 00007ffa`1d9543c4     mfplat!CMFInprocMFTLifetimeAssociation::~CMFInprocMFTLifetimeAssociation+0x4e
02 000000c2`21fbf100 00007ffa`1d8bfb00     mfplat!CMFInprocMFTLifetimeAssociation::`vector deleting destructor'+0x14
03 000000c2`21fbf130 00007ffa`38bc51d8     mfplat!Microsoft::WRL::Details::RuntimeClassImpl<Microsoft::WRL::RuntimeClassFlags<2>,1,0,0,IUnknown>::Release+0x30
04 000000c2`21fbf160 00007ffa`37ab4970     OLEAUT32!VariantClear+0x218
05 000000c2`21fbf1c0 00007ffa`1d8d2754     combase!PropVariantClearWorker+0x130
06 000000c2`21fbf1f0 00007ffa`1d8d27b5     mfplat!CPooledAttributes::Reset+0x34
07 000000c2`21fbf220 00007ffa`1d8d3314     mfplat!CPooledAttributes::_FinalRelease+0x25
08 000000c2`21fbf250 00007ff9`fc247dee     mfplat!CMFAttributes::Release+0x24
09 000000c2`21fbf280 00007ff9`fc2479c4     msvp9dec_store+0x7dee
0a 000000c2`21fbf2b0 00007ff9`fc255417     msvp9dec_store+0x79c4
0b 000000c2`21fbf2e0 00007ff9`c268f93a     msvp9dec_store+0x15417
0c (Inline Function) --------`--------     xul!mozilla::RefPtrTraits<IMFTransform>::Release+0xd
0d (Inline Function) --------`--------     xul!RefPtr<IMFTransform>::ConstRemovingRefPtrTraits<IMFTransform>::Release+0xd
0e (Inline Function) --------`--------     xul!RefPtr<IMFTransform>::~RefPtr+0x16
0f (Inline Function) --------`--------     xul!mozilla::MFTDecoder::~MFTDecoder+0x2c
10 000000c2`21fbf310 00007ff9`c268f8f9     xul!mozilla::MFTDecoder::Release+0x3a
11 (Inline Function) --------`--------     xul!mozilla::RefPtrTraits<mozilla::MFTDecoder>::Release+0x8
12 (Inline Function) --------`--------     xul!RefPtr<mozilla::MFTDecoder>::ConstRemovingRefPtrTraits<mozilla::MFTDecoder>::Release+0x8
13 (Inline Function) --------`--------     xul!RefPtr<mozilla::MFTDecoder>::~RefPtr+0x8
14 (Inline Function) --------`--------     xul!mozilla::CanCreateMFTDecoder::<unnamed-tag>::operator()+0x9d

And later, the worker thread unloads the module like this.

04 0000000f`0d2bf7d0 00007ffa`1d9589fa     KERNELBASE!FreeLibrary+0x1e
05 0000000f`0d2bf800 00007ffa`1d957ebd     mfplat!CMFInprocDllManager::TryUnloadDll+0x112
06 0000000f`0d2bf840 00007ffa`27e61632     mfplat!TMFWorkItem<<lambda_7e7b75702c9f908394152b2b7b5faf2a> >::Invoke+0xd
07 0000000f`0d2bf870 00007ffa`38ee1619     RTWorkQ!ThreadPoolTimerCallback+0x172
08 0000000f`0d2bf8f0 00007ffa`38ec315a     ntdll!TppTimerpExecuteCallback+0xa9
09 0000000f`0d2bf940 00007ffa`38cf7034     ntdll!TppWorkerThread+0x68a
0a 0000000f`0d2bfc40 00007ffa`38ec2651     KERNEL32!BaseThreadInitThunk+0x14
0b 0000000f`0d2bfc70 00000000`00000000     ntdll!RtlUserThreadStart+0x21

The problem is mfplat.dll posts the unload task several times while it loads the module only once. It's implemented in mfplat!CMFInprocDllManager::LoadDllInternal which calls LoadLibraryExW only when GetModuleHandleW returned null. As a result, even if we increment the module's refcount, the extra unload task could unload the module depending on timing.

Comment 1

[Toshihito Kikuchi [:toshi]]

Attachment: Bug 1723868 - Skip msvp9dec_store.dll in GetInfoForSelf(). r=gerald

When mfplat.dll loads msvp9dec_store.dll, it posts a task
to unload the module to the work queue even if msvp9dec_store.dll
is already loaded and mfplat.dll skips LoadLibrary. Therefore,
we cannot safely lock msvp9dec_store.dll by loading it as data.

The proposed fix is to skip processing the module.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/168c68c7f7e3
Skip msvp9dec_store.dll in GetInfoForSelf().  r=gerald

Comment 3

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/168c68c7f7e3

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for ESR91 approval when you get a chance.

Comment 5

[Toshihito Kikuchi [:toshi]]

Comment on attachment 9234752 [details]
Bug 1723868 - Skip msvp9dec_store.dll in GetInfoForSelf(). r=gerald

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This signature was a top crasher of RDD process. There were multiple root causes of this signature. I think this patch addresses the main root cause.
• User impact if declined: When RDD process loads Microsoft's built-in video decoder msvp9dec_store.dll, the process may suddenly crash even if no third-party application is installed. This is a timing issue.
• Fix Landed on Version: 92
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch skips msvp9dec_store.dll because we cannot keep it loaded. This is the same approach as we did for Nvidia driver (bug 1607574).
• String or UUID changes made by this patch: None

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9234752 [details]
Bug 1723868 - Skip msvp9dec_store.dll in GetInfoForSelf(). r=gerald

Approved for 91.1esr.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/38acd685ea4c