[Mac][MediaSource] Crash when SourceBuffer::provideMediaData() is called re-entrantly.
authorjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Apr 2015 04:31:45 +0000 (04:31 +0000)
committerjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Apr 2015 04:31:45 +0000 (04:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=144023

Reviewed by Darin Adler.

Partially revert r183097 (as it was not sufficient to protect against re-entrancy). Instead,
protect against re-entrancy in provideMediaData() directly by removing the first sample
from the TrackBuffer's decodeQueue at a time. If provideMediaData() is called re-entrantly,
or if any other method which modifies the decodeQueue is called from inside
provideMediaData, no iterators will be invalidated.

* Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::provideMediaData):
* platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
(WebCore::SourceBufferPrivateAVFObjC::didBecomeReadyForMoreSamples):

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

Source/WebCore/ChangeLog
Source/WebCore/Modules/mediasource/SourceBuffer.cpp
Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm

index a367277..5a2974f 100644 (file)
@@ -1,3 +1,21 @@
+2015-04-22  Jer Noble  <jer.noble@apple.com>
+
+        [Mac][MediaSource] Crash when SourceBuffer::provideMediaData() is called re-entrantly.
+        https://bugs.webkit.org/show_bug.cgi?id=144023
+
+        Reviewed by Darin Adler.
+
+        Partially revert r183097 (as it was not sufficient to protect against re-entrancy). Instead,
+        protect against re-entrancy in provideMediaData() directly by removing the first sample
+        from the TrackBuffer's decodeQueue at a time. If provideMediaData() is called re-entrantly,
+        or if any other method which modifies the decodeQueue is called from inside
+        provideMediaData, no iterators will be invalidated.
+
+        * Modules/mediasource/SourceBuffer.cpp:
+        (WebCore::SourceBuffer::provideMediaData):
+        * platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
+        (WebCore::SourceBufferPrivateAVFObjC::didBecomeReadyForMoreSamples):
+
 2015-04-22  Zalan Bujtas  <zalan@apple.com>
 
         Create RenderRubyText for <rt> only when the parent renderer is a RenderRuby.
index 7019c47..1b66577 100644 (file)
@@ -1793,14 +1793,18 @@ void SourceBuffer::provideMediaData(TrackBuffer& trackBuffer, AtomicString track
     unsigned enqueuedSamples = 0;
 #endif
 
-    auto sampleIt = trackBuffer.decodeQueue.begin();
-    for (auto sampleEnd = trackBuffer.decodeQueue.end(); sampleIt != sampleEnd; ++sampleIt) {
+    while (!trackBuffer.decodeQueue.empty()) {
         if (!m_private->isReadyForMoreSamples(trackID)) {
             m_private->notifyClientWhenReadyForMoreSamples(trackID);
             break;
         }
 
-        RefPtr<MediaSample> sample = sampleIt->second;
+        // FIXME(rdar://problem/20635969): Remove this re-entrancy protection when the aforementioned radar is resolved; protecting
+        // against re-entrancy introduces a small inefficency when removing appended samples from the decode queue one at a time
+        // rather than when all samples have been enqueued.
+        RefPtr<MediaSample> sample = trackBuffer.decodeQueue.begin()->second;
+        trackBuffer.decodeQueue.erase(trackBuffer.decodeQueue.begin());
+
         // Do not enqueue samples spanning a significant unbuffered gap.
         // NOTE: one second is somewhat arbitrary. MediaSource::monitorSourceBuffers() is run
         // on the playbackTimer, which is effectively every 350ms. Allowing > 350ms gap between
@@ -1818,9 +1822,7 @@ void SourceBuffer::provideMediaData(TrackBuffer& trackBuffer, AtomicString track
 #if !LOG_DISABLED
         ++enqueuedSamples;
 #endif
-
     }
-    trackBuffer.decodeQueue.erase(trackBuffer.decodeQueue.begin(), sampleIt);
 
     LOG(MediaSource, "SourceBuffer::provideMediaData(%p) - Enqueued %u samples", this, enqueuedSamples);
 }
index 3b590c9..3a54122 100644 (file)
@@ -1087,13 +1087,8 @@ void SourceBufferPrivateAVFObjC::didBecomeReadyForMoreSamples(int trackID)
         return;
     }
 
-    // FIXME(rdar://problem/20635969): Remove this dispatch_async() when the aforementioned radar is resolved
-    auto weakThis = createWeakPtr();
-    dispatch_async(dispatch_get_main_queue(), [weakThis, trackID] {
-        if (!weakThis || !weakThis->m_client)
-            return;
-        weakThis->m_client->sourceBufferPrivateDidBecomeReadyForMoreSamples(weakThis.get(), AtomicString::number(trackID));
-    });
+    if (m_client)
+        m_client->sourceBufferPrivateDidBecomeReadyForMoreSamples(this, AtomicString::number(trackID));
 }
 
 void SourceBufferPrivateAVFObjC::notifyClientWhenReadyForMoreSamples(AtomicString trackIDString)