Bug 239485

Summary: [WinCairo] Crash while MediaPlayerPrivateMediaFoundation::removeListener in the async callback thread
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: PlatformAssignee: Fujii Hironori <Hironori.Fujii>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, don.olmstead, eric.carlson, ews-watchlist, glenn, jer.noble, philipj, pvollan, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
crash log
none
Patch none

Description Fujii Hironori 2022-04-18 23:50:09 PDT 
Created attachment 457861 [details]
crash log

[WinCairo] Crash while MediaPlayerPrivateMediaFoundation::removeListener in the async callback thread

WinCairo Debug WK2 r292978 

Callstack:

> WebKit2!WTF::HashTable<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *,WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *,WTF::IdentityExtractor,WTF::DefaultHash<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::HashTraits<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::HashTraits<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *> >::tableSizeMask(void)+0x85 [C:\jenkins_slave\WinCairo-master\WebKitBuild\Debug\WTF\Headers\wtf\HashTable.h @ 598]
> WebKit2!WTF::HashTable<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *,WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *,WTF::IdentityExtractor,WTF::DefaultHash<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::HashTraits<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::HashTraits<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *> >::inlineLookup<WTF::IdentityHashTranslator<WTF::HashTraits<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::DefaultHash<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *> >,WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>(class WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener ** key = 0x0000002a`4f8ff7f8)+0x4c [C:\jenkins_slave\WinCairo-master\WebKitBuild\Debug\WTF\Headers\wtf\HashTable.h @ 692]
> WebKit2!WTF::HashTable<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *,WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *,WTF::IdentityExtractor,WTF::DefaultHash<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::HashTraits<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::HashTraits<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *> >::lookup<WTF::IdentityHashTranslator<WTF::HashTraits<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::DefaultHash<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *> >,WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>(class WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener ** key = 0x0000002a`4f8ff7f8)+0x1e [C:\jenkins_slave\WinCairo-master\WebKitBuild\Debug\WTF\Headers\wtf\HashTable.h @ 678]
> WebKit2!WTF::HashTable<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *,WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *,WTF::IdentityExtractor,WTF::DefaultHash<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::HashTraits<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::HashTraits<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *> >::find<WTF::IdentityHashTranslator<WTF::HashTraits<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::DefaultHash<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *> >,WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>(class WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener ** key = 0x0000002a`4f8ff7f8)+0x44 [C:\jenkins_slave\WinCairo-master\WebKitBuild\Debug\WTF\Headers\wtf\HashTable.h @ 1101]
> WebKit2!WTF::HashTable<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *,WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *,WTF::IdentityExtractor,WTF::DefaultHash<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::HashTraits<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::HashTraits<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *> >::find(class WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener ** key = 0x0000002a`4f8ff7f8)+0x28 [C:\jenkins_slave\WinCairo-master\WebKitBuild\Debug\WTF\Headers\wtf\HashTable.h @ 493]
> WebKit2!WTF::HashSet<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *,WTF::DefaultHash<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::HashTraits<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::HashTableTraits>::find(class WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener ** value = 0x0000002a`4f8ff7f8)+0x32 [C:\jenkins_slave\WinCairo-master\WebKitBuild\Debug\WTF\Headers\wtf\HashSet.h @ 234]
> WebKit2!WTF::HashSet<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *,WTF::DefaultHash<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::HashTraits<WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener *>,WTF::HashTableTraits>::remove(class WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener ** value = 0x0000002a`4f8ff7f8)+0x2d [C:\jenkins_slave\WinCairo-master\WebKitBuild\Debug\WTF\Headers\wtf\HashSet.h @ 328]
> WebKit2!WebCore::MediaPlayerPrivateMediaFoundation::removeListener(class WebCore::MediaPlayerPrivateMediaFoundation::MediaPlayerListener * listener = 0x000001bc`dfbe6ce8)+0x67 [C:\jenkins_slave\WinCairo-master\Source\WebCore\platform\graphics\win\MediaPlayerPrivateMediaFoundation.cpp @ 655]
> WebKit2!WebCore::MediaPlayerPrivateMediaFoundation::AsyncCallback::~AsyncCallback(void)+0x73 [C:\jenkins_slave\WinCairo-master\Source\WebCore\platform\graphics\win\MediaPlayerPrivateMediaFoundation.cpp @ 872]
> WebKit2!WebCore::MediaPlayerPrivateMediaFoundation::AsyncCallback::`scalar deleting destructor'(void)+0x18
> WebKit2!WebCore::MediaPlayerPrivateMediaFoundation::AsyncCallback::Release(void)+0x6e [C:\jenkins_slave\WinCairo-master\Source\WebCore\platform\graphics\win\MediaPlayerPrivateMediaFoundation.cpp @ 898]
> MFPlat!CAsyncResult::~CAsyncResult+0x34
> MFPlat!CResolverResult::`vector deleting destructor'+0x14
> MFPlat!CPoolableObject::_FinalRelease+0x1a
> MFPlat!CResolverResult::Release+0x2d
> RTWorkQ!CSerialWorkQueue::ProcessNextItem+0x278
> RTWorkQ!CSerialWorkQueue::QueueItem::OnWorkItemAsyncCallback::Invoke+0x96
> RTWorkQ!ThreadPoolWorkCallback+0xbd
> ntdll!TppWorkpExecuteCallback+0x130
> ntdll!TppWorkerThread+0x68a
> KERNEL32!BaseThreadInitThunk+0x14
> ntdll!RtlUserThreadStart+0x21
Comment 1 Fujii Hironori 2022-04-19 00:02:29 PDT 
I don't know how to reproduce this crash and what is the reason.
But, looking at the code, it seems that there is a problem.

On the main thread, m_mediaPlayer is cleared with locking m_mutex.

> void MediaPlayerPrivateMediaFoundation::AsyncCallback::onMediaPlayerDeleted()
> {
>     Locker locker { m_mutex };
> 
>     m_mediaPlayer = nullptr;
> }

However, m_mediaPlayer is accessed without locking the mutex in the async callback thread.

> MediaPlayerPrivateMediaFoundation::AsyncCallback::~AsyncCallback()
> {
>     if (m_mediaPlayer)
>         m_mediaPlayer->removeListener(this);
> }
Comment 2 Fujii Hironori 2022-04-26 13:36:49 PDT 
One more problem.
IMFAsyncCallback should use InterlockedIncrement and InterlockedDecrement for ref-counting.

Implementing the Asynchronous Callback - Win32 apps | Microsoft Docs
https://docs.microsoft.com/en-us/windows/win32/medfound/implementing-the-asynchronous-callback
Comment 3 Fujii Hironori 2022-04-26 15:18:16 PDT 
Created attachment 458406 [details]
Patch
Comment 4 EWS 2022-05-04 01:33:06 PDT 
Committed r293765 (250244@main): <https://commits.webkit.org/250244@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 458406 [details].
Comment 5 Radar WebKit Bug Importer 2022-05-04 01:34:13 PDT 
<rdar://problem/92719665>