Modified: trunk/Source/WebCore/ChangeLog (214226 => 214227)
--- trunk/Source/WebCore/ChangeLog 2017-03-21 18:18:35 UTC (rev 214226)
+++ trunk/Source/WebCore/ChangeLog 2017-03-21 19:45:38 UTC (rev 214227)
@@ -1,3 +1,23 @@
+2017-03-20 Jer Noble <jer.no...@apple.com>
+
+ Crash in WebCore: WebCore::CARingBuffer::getCurrentFrameBounds + 28
+ https://bugs.webkit.org/show_bug.cgi?id=169887
+ <rdar://problem/23648082>
+
+ Reviewed by Eric Carlson.
+
+ Because AudioSourceProviderAVFObjC::prepareCallback() can concievably be called after the AudioSourceProviderAVFObjC
+ it refers to has been destroyed, add an extra layer of indirection between the tap and the provider, and invalidate
+ that level of indirection before the AudioSourceProviderAVFObjC is destroyed.
+
+ * platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h:
+ * platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm:
+ (WebCore::AudioSourceProviderAVFObjC::initCallback):
+ (WebCore::AudioSourceProviderAVFObjC::finalizeCallback):
+ (WebCore::AudioSourceProviderAVFObjC::prepareCallback):
+ (WebCore::AudioSourceProviderAVFObjC::unprepareCallback):
+ (WebCore::AudioSourceProviderAVFObjC::processCallback):
+
2017-03-21 Simon Fraser <simon.fra...@apple.com>
Remove logging left in by mistake.
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h (214226 => 214227)
--- trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h 2017-03-21 18:18:35 UTC (rev 214226)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h 2017-03-21 19:45:38 UTC (rev 214227)
@@ -30,7 +30,6 @@
#include "AudioSourceProvider.h"
#include <atomic>
-#include <wtf/Lock.h>
#include <wtf/MediaTime.h>
#include <wtf/RefCounted.h>
#include <wtf/RetainPtr.h>
@@ -90,7 +89,6 @@
std::unique_ptr<AudioStreamBasicDescription> m_outputDescription;
std::unique_ptr<CARingBuffer> m_ringBuffer;
- Lock m_mutex;
MediaTime m_startTimeAtLastProcess;
MediaTime m_endTimeAtLastProcess;
std::atomic<uint64_t> m_writeAheadCount { 0 };
@@ -99,6 +97,9 @@
std::atomic<uint64_t> m_seekTo { NoSeek };
bool m_paused { true };
AudioSourceProviderClient* m_client { nullptr };
+
+ struct TapStorage;
+ TapStorage* m_tapStorage { nullptr };
};
}
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm (214226 => 214227)
--- trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm 2017-03-21 18:18:35 UTC (rev 214226)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm 2017-03-21 19:45:38 UTC (rev 214227)
@@ -40,6 +40,7 @@
#import <AVFoundation/AVPlayerItem.h>
#import <mutex>
#import <objc/runtime.h>
+#import <wtf/Lock.h>
#import <wtf/MainThread.h>
#if !LOG_DISABLED
@@ -70,6 +71,12 @@
static const double kRingBufferDuration = 1;
+struct AudioSourceProviderAVFObjC::TapStorage {
+ TapStorage(AudioSourceProviderAVFObjC* _this) : _this(_this) { }
+ AudioSourceProviderAVFObjC* _this;
+ Lock mutex;
+};
+
RefPtr<AudioSourceProviderAVFObjC> AudioSourceProviderAVFObjC::create(AVPlayerItem *item)
{
if (!canLoadMTAudioProcessingTapCreate())
@@ -85,6 +92,11 @@
AudioSourceProviderAVFObjC::~AudioSourceProviderAVFObjC()
{
setClient(nullptr);
+ if (m_tapStorage) {
+ std::unique_lock<Lock> lock(m_tapStorage->mutex);
+ m_tapStorage->_this = nullptr;
+ m_tapStorage = nullptr;
+ }
}
void AudioSourceProviderAVFObjC::provideInput(AudioBus* bus, size_t framesToProcess)
@@ -92,7 +104,12 @@
// Protect access to m_ringBuffer by try_locking the mutex. If we failed
// to aquire, a re-configure is underway, and m_ringBuffer is unsafe to access.
// Emit silence.
- std::unique_lock<Lock> lock(m_mutex, std::try_to_lock);
+ if (!m_tapStorage) {
+ bus->zero();
+ return;
+ }
+
+ std::unique_lock<Lock> lock(m_tapStorage->mutex, std::try_to_lock);
if (!lock.owns_lock() || !m_ringBuffer) {
bus->zero();
return;
@@ -222,35 +239,54 @@
{
AudioSourceProviderAVFObjC* _this = static_cast<AudioSourceProviderAVFObjC*>(clientInfo);
_this->m_tap = tap;
+ _this->m_tapStorage = new TapStorage(_this);
_this->init(clientInfo, tapStorageOut);
+ *tapStorageOut = _this->m_tapStorage;
}
void AudioSourceProviderAVFObjC::finalizeCallback(MTAudioProcessingTapRef tap)
{
ASSERT(tap);
- AudioSourceProviderAVFObjC* _this = static_cast<AudioSourceProviderAVFObjC*>(MTAudioProcessingTapGetStorage(tap));
- _this->finalize();
+ TapStorage* tapStorage = static_cast<TapStorage*>(MTAudioProcessingTapGetStorage(tap));
+
+ std::lock_guard<Lock> lock(tapStorage->mutex);
+
+ if (tapStorage->_this)
+ tapStorage->_this->finalize();
+ delete tapStorage;
}
void AudioSourceProviderAVFObjC::prepareCallback(MTAudioProcessingTapRef tap, CMItemCount maxFrames, const AudioStreamBasicDescription *processingFormat)
{
ASSERT(tap);
- AudioSourceProviderAVFObjC* _this = static_cast<AudioSourceProviderAVFObjC*>(MTAudioProcessingTapGetStorage(tap));
- _this->prepare(maxFrames, processingFormat);
+ TapStorage* tapStorage = static_cast<TapStorage*>(MTAudioProcessingTapGetStorage(tap));
+
+ std::lock_guard<Lock> lock(tapStorage->mutex);
+
+ if (tapStorage->_this)
+ tapStorage->_this->prepare(maxFrames, processingFormat);
}
void AudioSourceProviderAVFObjC::unprepareCallback(MTAudioProcessingTapRef tap)
{
ASSERT(tap);
- AudioSourceProviderAVFObjC* _this = static_cast<AudioSourceProviderAVFObjC*>(MTAudioProcessingTapGetStorage(tap));
- _this->unprepare();
+ TapStorage* tapStorage = static_cast<TapStorage*>(MTAudioProcessingTapGetStorage(tap));
+
+ std::lock_guard<Lock> lock(tapStorage->mutex);
+
+ if (tapStorage->_this)
+ tapStorage->_this->unprepare();
}
void AudioSourceProviderAVFObjC::processCallback(MTAudioProcessingTapRef tap, CMItemCount numberFrames, MTAudioProcessingTapFlags flags, AudioBufferList *bufferListInOut, CMItemCount *numberFramesOut, MTAudioProcessingTapFlags *flagsOut)
{
ASSERT(tap);
- AudioSourceProviderAVFObjC* _this = static_cast<AudioSourceProviderAVFObjC*>(MTAudioProcessingTapGetStorage(tap));
- _this->process(numberFrames, flags, bufferListInOut, numberFramesOut, flagsOut);
+ TapStorage* tapStorage = static_cast<TapStorage*>(MTAudioProcessingTapGetStorage(tap));
+
+ std::lock_guard<Lock> lock(tapStorage->mutex);
+
+ if (tapStorage->_this)
+ tapStorage->_this->process(numberFrames, flags, bufferListInOut, numberFramesOut, flagsOut);
}
void AudioSourceProviderAVFObjC::init(void* clientInfo, void** tapStorageOut)
@@ -262,6 +298,10 @@
void AudioSourceProviderAVFObjC::finalize()
{
+ if (m_tapStorage) {
+ m_tapStorage->_this = nullptr;
+ m_tapStorage = nullptr;
+ }
}
void AudioSourceProviderAVFObjC::prepare(CMItemCount maxFrames, const AudioStreamBasicDescription *processingFormat)
@@ -268,8 +308,6 @@
{
ASSERT(maxFrames >= 0);
- std::lock_guard<Lock> lock(m_mutex);
-
m_tapDescription = std::make_unique<AudioStreamBasicDescription>(*processingFormat);
int numberOfChannels = processingFormat->mChannelsPerFrame;
double sampleRate = processingFormat->mSampleRate;
@@ -312,8 +350,6 @@
void AudioSourceProviderAVFObjC::unprepare()
{
- std::lock_guard<Lock> lock(m_mutex);
-
m_tapDescription = nullptr;
m_outputDescription = nullptr;
m_ringBuffer = nullptr;
