[Win] Avoid deadlock when interacting with some AVFoundationCF content
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Nov 2013 17:23:21 +0000 (17:23 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Nov 2013 17:23:21 +0000 (17:23 +0000)
<rdar://problem/15482977> and https://bugs.webkit.org/show_bug.cgi?id=124752

Prevent deadlock caused by conflict over the "mapLock" mutex. Notification handling in the file,
which modify assets and make other changes, are required to happen on the main thread. This
patch enforces this requirement.

Reviewed by Eric Carlson.

* platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp:
(WebCore::NotificationCallbackData::NotificationCallbackData): Added
(WebCore::AVFWrapper::processNotification): Moved logic from 'notificationCallback', which was
sometimes happening on a background thread.
(WebCore::AVFWrapper::notificationCallback): Dispatch calls to main thread.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@159691 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp

index 4460ca8241f0f8397b9a62603965a8f110f50aef..b94c5f828719450f9f7ef62569283d606892a5e8 100644 (file)
@@ -1,3 +1,20 @@
+2013-11-22  Brent Fulgham  <bfulgham@apple.com>
+
+        [Win] Avoid deadlock when interacting with some AVFoundationCF content
+        <rdar://problem/15482977> and https://bugs.webkit.org/show_bug.cgi?id=124752
+
+        Prevent deadlock caused by conflict over the "mapLock" mutex. Notification handling in the file,
+        which modify assets and make other changes, are required to happen on the main thread. This
+        patch enforces this requirement.
+
+        Reviewed by Eric Carlson.
+
+        * platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp:
+        (WebCore::NotificationCallbackData::NotificationCallbackData): Added
+        (WebCore::AVFWrapper::processNotification): Moved logic from 'notificationCallback', which was
+        sometimes happening on a background thread.
+        (WebCore::AVFWrapper::notificationCallback): Dispatch calls to main thread.
+
 2013-11-22  peavo@outlook.com  <peavo@outlook.com>
 
         [WinCairo] Compile error when ACCELERATED_COMPOSITING is not used.
index 90ed705d8b164460885824681a3c825e4fdfaf8a..17e132b0098eb5f8790c0bc933f83d710412715d 100644 (file)
@@ -122,6 +122,7 @@ public:
     static void periodicTimeObserverCallback(AVCFPlayerRef, CMTime, void*);
     static void seekCompletedCallback(AVCFPlayerItemRef, Boolean, void*);
     static void notificationCallback(CFNotificationCenterRef, void*, CFStringRef, const void*, CFDictionaryRef);
+    static void processNotification(void* context);
 
     inline AVCFPlayerLayerRef videoLayer() const { return (AVCFPlayerLayerRef)m_avCFVideoLayer.get(); }
     inline AVCFPlayerRef avPlayer() const { return (AVCFPlayerRef)m_avPlayer.get(); }
@@ -1410,21 +1411,34 @@ void AVFWrapper::periodicTimeObserverCallback(AVCFPlayerRef, CMTime cmTime, void
     self->m_owner->scheduleMainThreadNotification(MediaPlayerPrivateAVFoundation::Notification::PlayerTimeChanged, time);
 }
 
-void AVFWrapper::notificationCallback(CFNotificationCenterRef, void* observer, CFStringRef propertyName, const void* object, CFDictionaryRef)
+struct NotificationCallbackData {
+    RetainPtr<CFStringRef> m_propertyName;
+    void* m_context;
+
+    NotificationCallbackData(CFStringRef propertyName, void* context)
+        : m_propertyName(propertyName), m_context(context)
+    {
+    }
+};
+
+void AVFWrapper::processNotification(void* context)
 {
+    ASSERT(dispatch_get_main_queue() == dispatch_get_current_queue());
+    ASSERT(context);
+
+    if (!context)
+        return;
+
+    OwnPtr<NotificationCallbackData> notificationData = adoptPtr(reinterpret_cast<NotificationCallbackData*>(context));
+
     MutexLocker locker(mapLock());
-    AVFWrapper* self = avfWrapperForCallbackContext(observer);
-    
+    AVFWrapper* self = avfWrapperForCallbackContext(notificationData->m_context);
     if (!self) {
-        LOG(Media, "AVFWrapper::notificationCallback invoked for deleted AVFWrapper %d", reinterpret_cast<uintptr_t>(observer));
+        LOG(Media, "AVFWrapper::processNotification invoked for deleted AVFWrapper %d", reinterpret_cast<uintptr_t>(context));
         return;
     }
 
-#if !LOG_DISABLED
-    char notificationName[256];
-    CFStringGetCString(propertyName, notificationName, sizeof(notificationName), kCFStringEncodingASCII);
-    LOG(Media, "AVFWrapper::notificationCallback(%p) %s", self, notificationName);
-#endif
+    CFStringRef propertyName = notificationData->m_propertyName.get();
 
     if (CFEqual(propertyName, AVCFPlayerItemDidPlayToEndTimeNotification))
         self->m_owner->scheduleMainThreadNotification(MediaPlayerPrivateAVFoundation::Notification::ItemDidPlayToEndTime);
@@ -1455,6 +1469,19 @@ void AVFWrapper::notificationCallback(CFNotificationCenterRef, void* observer, C
         ASSERT_NOT_REACHED();
 }
 
+void AVFWrapper::notificationCallback(CFNotificationCenterRef, void* observer, CFStringRef propertyName, const void* object, CFDictionaryRef)
+{
+#if !LOG_DISABLED
+    char notificationName[256];
+    CFStringGetCString(propertyName, notificationName, sizeof(notificationName), kCFStringEncodingASCII);
+    LOG(Media, "AVFWrapper::notificationCallback(if=%d) %s", reinterpret_cast<uintptr_t>(observer), notificationName);
+#endif
+
+    OwnPtr<NotificationCallbackData> notificationData = adoptPtr(new NotificationCallbackData(propertyName, observer));
+
+    dispatch_async_f(dispatch_get_main_queue(), notificationData.leakPtr(), processNotification);
+}
+
 void AVFWrapper::loadPlayableCompletionCallback(AVCFAssetRef, void* context)
 {
     MutexLocker locker(mapLock());