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

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Fujii Hironori]

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

[Fujii Hironori]

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);
> }

[Fujii Hironori]

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

[Fujii Hironori]

Created attachment 458406 [details]
Patch

[EWS]

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

[Radar WebKit Bug Importer]

<rdar://problem/92719665>