Title: [217878] trunk/Source/WebCore
Revision
217878
Author
pvol...@apple.com
Date
2017-06-07 03:57:23 -0700 (Wed, 07 Jun 2017)

Log Message

AudioSourceProviderAVFObjC::m_tap member access is not thread safe.
https://bugs.webkit.org/show_bug.cgi?id=172263

Reviewed by Darin Adler.

It it possible that the main thread will modify the m_tap member by calling
AudioSourceProviderAVFObjC::destroyMix(), while m_tap is being read on a secondary
thread under AudioSourceProviderAVFObjC::processCallback(). Instead of accessing
the m_tap member on the secondary thread, we can use the same MTAudioProcessingTapRef
provided by MediaToolbox in the callback. We assume the tap ref is retained by
MediaToolbox, so it should be safe to use even if the the m_tap member is set to null.

Covered by existing tests.

* platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h:
* platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm:
(WebCore::AudioSourceProviderAVFObjC::processCallback):
(WebCore::AudioSourceProviderAVFObjC::process):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (217877 => 217878)


--- trunk/Source/WebCore/ChangeLog	2017-06-07 09:16:03 UTC (rev 217877)
+++ trunk/Source/WebCore/ChangeLog	2017-06-07 10:57:23 UTC (rev 217878)
@@ -1,3 +1,24 @@
+2017-06-07  Per Arne Vollan  <pvol...@apple.com>
+
+        AudioSourceProviderAVFObjC::m_tap member access is not thread safe.
+        https://bugs.webkit.org/show_bug.cgi?id=172263
+
+        Reviewed by Darin Adler.
+
+        It it possible that the main thread will modify the m_tap member by calling
+        AudioSourceProviderAVFObjC::destroyMix(), while m_tap is being read on a secondary
+        thread under AudioSourceProviderAVFObjC::processCallback(). Instead of accessing
+        the m_tap member on the secondary thread, we can use the same MTAudioProcessingTapRef
+        provided by MediaToolbox in the callback. We assume the tap ref is retained by
+        MediaToolbox, so it should be safe to use even if the the m_tap member is set to null.
+
+        Covered by existing tests.
+
+        * platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h:
+        * platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm:
+        (WebCore::AudioSourceProviderAVFObjC::processCallback):
+        (WebCore::AudioSourceProviderAVFObjC::process):
+
 2017-06-07  Zan Dobersek  <zdober...@igalia.com>
 
         [GCrypt] RSA-PSS support

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h (217877 => 217878)


--- trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h	2017-06-07 09:16:03 UTC (rev 217877)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h	2017-06-07 10:57:23 UTC (rev 217878)
@@ -77,7 +77,7 @@
     void finalize();
     void prepare(CMItemCount maxFrames, const AudioStreamBasicDescription *processingFormat);
     void unprepare();
-    void process(CMItemCount numberFrames, MTAudioProcessingTapFlags flagsIn, AudioBufferList *bufferListInOut, CMItemCount *numberFramesOut, MTAudioProcessingTapFlags *flagsOut);
+    void process(MTAudioProcessingTapRef, CMItemCount numberFrames, MTAudioProcessingTapFlags flagsIn, AudioBufferList *bufferListInOut, CMItemCount *numberFramesOut, MTAudioProcessingTapFlags *flagsOut);
 
     RetainPtr<AVPlayerItem> m_avPlayerItem;
     RetainPtr<AVAssetTrack> m_avAssetTrack;

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm (217877 => 217878)


--- trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm	2017-06-07 09:16:03 UTC (rev 217877)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm	2017-06-07 10:57:23 UTC (rev 217878)
@@ -287,7 +287,7 @@
     std::lock_guard<Lock> lock(tapStorage->mutex);
 
     if (tapStorage->_this)
-        tapStorage->_this->process(numberFrames, flags, bufferListInOut, numberFramesOut, flagsOut);
+        tapStorage->_this->process(tap, numberFrames, flags, bufferListInOut, numberFramesOut, flagsOut);
 }
 
 void AudioSourceProviderAVFObjC::init(void* clientInfo, void** tapStorageOut)
@@ -357,17 +357,13 @@
     m_list = nullptr;
 }
 
-void AudioSourceProviderAVFObjC::process(CMItemCount numberOfFrames, MTAudioProcessingTapFlags flags, AudioBufferList* bufferListInOut, CMItemCount* numberFramesOut, MTAudioProcessingTapFlags* flagsOut)
+void AudioSourceProviderAVFObjC::process(MTAudioProcessingTapRef tap, CMItemCount numberOfFrames, MTAudioProcessingTapFlags flags, AudioBufferList* bufferListInOut, CMItemCount* numberFramesOut, MTAudioProcessingTapFlags* flagsOut)
 {
     UNUSED_PARAM(flags);
     
-    RetainPtr<MTAudioProcessingTapRef> tap = m_tap;
-    if (!tap)
-        return;
-
     CMItemCount itemCount = 0;
     CMTimeRange rangeOut;
-    OSStatus status = MTAudioProcessingTapGetSourceAudio(tap.get(), numberOfFrames, bufferListInOut, flagsOut, &rangeOut, &itemCount);
+    OSStatus status = MTAudioProcessingTapGetSourceAudio(tap, numberOfFrames, bufferListInOut, flagsOut, &rangeOut, &itemCount);
     if (status != noErr || !itemCount)
         return;
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to