<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[198654] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/198654">198654</a></dd>
<dt>Author</dt> <dd>jer.noble@apple.com</dd>
<dt>Date</dt> <dd>2016-03-24 17:52:40 -0700 (Thu, 24 Mar 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Safari Crashes if audio.src is changed while connected to AudioAnalyserNode
https://bugs.webkit.org/show_bug.cgi?id=153593
<rdar://problem/23648082>
Reviewed by Eric Carlson.
m_ringBuffer is accessed on the high-priority WebAudio thread after it has been cleared (a
null-deref). Protect against unsafe access on multiple threads of a non-refcounted object by
a simple try_lock.
Additionally, limit the use of variables in use by both the separate WebAudio thread method
(provideInput()) and AVAudioMix thread method (process()) where possible, and convert to
std::atomic<> where ivars must be acessed by both threads. m_writeCount is entirely superfluous,
as it is a synonym for the endTime returned by m_ringBuffer->getCurrentFrameBounds().
* platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h:
* platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm:
(WebCore::AudioSourceProviderAVFObjC::provideInput):
(WebCore::AudioSourceProviderAVFObjC::prepare):
(WebCore::AudioSourceProviderAVFObjC::unprepare):
(WebCore::AudioSourceProviderAVFObjC::process):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsavfoundationAudioSourceProviderAVFObjCh">trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsavfoundationAudioSourceProviderAVFObjCmm">trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (198653 => 198654)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-03-24 23:56:27 UTC (rev 198653)
+++ trunk/Source/WebCore/ChangeLog 2016-03-25 00:52:40 UTC (rev 198654)
</span><span class="lines">@@ -1,3 +1,27 @@
</span><ins>+2016-03-24 Jer Noble <jer.noble@apple.com>
+
+ Safari Crashes if audio.src is changed while connected to AudioAnalyserNode
+ https://bugs.webkit.org/show_bug.cgi?id=153593
+ <rdar://problem/23648082>
+
+ Reviewed by Eric Carlson.
+
+ m_ringBuffer is accessed on the high-priority WebAudio thread after it has been cleared (a
+ null-deref). Protect against unsafe access on multiple threads of a non-refcounted object by
+ a simple try_lock.
+
+ Additionally, limit the use of variables in use by both the separate WebAudio thread method
+ (provideInput()) and AVAudioMix thread method (process()) where possible, and convert to
+ std::atomic<> where ivars must be acessed by both threads. m_writeCount is entirely superfluous,
+ as it is a synonym for the endTime returned by m_ringBuffer->getCurrentFrameBounds().
+
+ * platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h:
+ * platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm:
+ (WebCore::AudioSourceProviderAVFObjC::provideInput):
+ (WebCore::AudioSourceProviderAVFObjC::prepare):
+ (WebCore::AudioSourceProviderAVFObjC::unprepare):
+ (WebCore::AudioSourceProviderAVFObjC::process):
+
</ins><span class="cx"> 2016-03-24 Enrica Casucci <enrica@apple.com>
</span><span class="cx">
</span><span class="cx"> Adopt new SPI from DataDetectorsCore to decide link behavior.
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsavfoundationAudioSourceProviderAVFObjCh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h (198653 => 198654)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h 2016-03-24 23:56:27 UTC (rev 198653)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h 2016-03-25 00:52:40 UTC (rev 198654)
</span><span class="lines">@@ -29,6 +29,8 @@
</span><span class="cx"> #if ENABLE(WEB_AUDIO) && USE(MEDIATOOLBOX)
</span><span class="cx">
</span><span class="cx"> #include "AudioSourceProvider.h"
</span><ins>+#include <atomic>
+#include <wtf/Lock.h>
</ins><span class="cx"> #include <wtf/MediaTime.h>
</span><span class="cx"> #include <wtf/RefCounted.h>
</span><span class="cx"> #include <wtf/RefPtr.h>
</span><span class="lines">@@ -89,13 +91,15 @@
</span><span class="cx"> std::unique_ptr<AudioStreamBasicDescription> m_outputDescription;
</span><span class="cx"> std::unique_ptr<CARingBuffer> m_ringBuffer;
</span><span class="cx">
</span><ins>+ Lock m_mutex;
</ins><span class="cx"> MediaTime m_startTimeAtLastProcess;
</span><span class="cx"> MediaTime m_endTimeAtLastProcess;
</span><del>- uint64_t m_writeAheadCount;
- uint64_t m_writeCount;
- uint64_t m_readCount;
- bool m_paused;
- AudioSourceProviderClient* m_client;
</del><ins>+ std::atomic<uint64_t> m_writeAheadCount { 0 };
+ uint64_t m_readCount { 0 };
+ enum { NoSeek = std::numeric_limits<uint64_t>::max() };
+ std::atomic<uint64_t> m_seekTo { NoSeek };
+ bool m_paused { true };
+ AudioSourceProviderClient* m_client { nullptr };
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsavfoundationAudioSourceProviderAVFObjCmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm (198653 => 198654)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm 2016-03-24 23:56:27 UTC (rev 198653)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm 2016-03-25 00:52:40 UTC (rev 198654)
</span><span class="lines">@@ -38,6 +38,7 @@
</span><span class="cx"> #import <AVFoundation/AVAudioMix.h>
</span><span class="cx"> #import <AVFoundation/AVMediaFormat.h>
</span><span class="cx"> #import <AVFoundation/AVPlayerItem.h>
</span><ins>+#import <mutex>
</ins><span class="cx"> #import <objc/runtime.h>
</span><span class="cx"> #import <wtf/MainThread.h>
</span><span class="cx">
</span><span class="lines">@@ -78,10 +79,6 @@
</span><span class="cx">
</span><span class="cx"> AudioSourceProviderAVFObjC::AudioSourceProviderAVFObjC(AVPlayerItem *item)
</span><span class="cx"> : m_avPlayerItem(item)
</span><del>- , m_writeCount(0)
- , m_readCount(0)
- , m_paused(true)
- , m_client(nullptr)
</del><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -92,19 +89,30 @@
</span><span class="cx">
</span><span class="cx"> void AudioSourceProviderAVFObjC::provideInput(AudioBus* bus, size_t framesToProcess)
</span><span class="cx"> {
</span><del>- if (!m_avPlayerItem)
</del><ins>+ // 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 (!lock.owns_lock() || !m_ringBuffer) {
+ bus->zero();
</ins><span class="cx"> return;
</span><ins>+ }
</ins><span class="cx">
</span><span class="cx"> uint64_t startFrame = 0;
</span><span class="cx"> uint64_t endFrame = 0;
</span><ins>+ uint64_t seekTo = m_seekTo.exchange(NoSeek);
+ uint64_t writeAheadCount = m_writeAheadCount.load();
+ if (seekTo != NoSeek)
+ m_readCount = seekTo;
+
</ins><span class="cx"> m_ringBuffer->getCurrentFrameBounds(startFrame, endFrame);
</span><span class="cx">
</span><del>- if (m_writeCount <= m_readCount + m_writeAheadCount) {
</del><ins>+ size_t framesAvailable = static_cast<size_t>(endFrame - (m_readCount + writeAheadCount));
+ if (!framesAvailable) {
</ins><span class="cx"> bus->zero();
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- size_t framesAvailable = static_cast<size_t>(endFrame - (m_readCount + m_writeAheadCount));
</del><span class="cx"> if (framesAvailable < framesToProcess) {
</span><span class="cx"> framesToProcess = framesAvailable;
</span><span class="cx"> bus->zero();
</span><span class="lines">@@ -277,6 +285,8 @@
</span><span class="cx"> {
</span><span class="cx"> ASSERT(maxFrames >= 0);
</span><span class="cx">
</span><ins>+ std::lock_guard<Lock> lock(m_mutex);
+
</ins><span class="cx"> m_tapDescription = std::make_unique<AudioStreamBasicDescription>(*processingFormat);
</span><span class="cx"> int numberOfChannels = processingFormat->mChannelsPerFrame;
</span><span class="cx"> size_t bytesPerFrame = processingFormat->mBytesPerFrame;
</span><span class="lines">@@ -321,6 +331,8 @@
</span><span class="cx">
</span><span class="cx"> void AudioSourceProviderAVFObjC::unprepare()
</span><span class="cx"> {
</span><ins>+ std::lock_guard<Lock> lock(m_mutex);
+
</ins><span class="cx"> m_tapDescription = nullptr;
</span><span class="cx"> m_outputDescription = nullptr;
</span><span class="cx"> m_ringBuffer = nullptr;
</span><span class="lines">@@ -358,13 +370,17 @@
</span><span class="cx"> // Only check the write-ahead time when playback begins.
</span><span class="cx"> m_paused = false;
</span><span class="cx"> MediaTime earlyBy = rangeStart - currentTime;
</span><del>- m_writeAheadCount = m_tapDescription->mSampleRate * earlyBy.toDouble();
</del><ins>+ m_writeAheadCount.store(m_tapDescription->mSampleRate * earlyBy.toDouble());
</ins><span class="cx"> }
</span><span class="cx">
</span><ins>+ uint64_t startFrame = 0;
+ uint64_t endFrame = 0;
+ m_ringBuffer->getCurrentFrameBounds(startFrame, endFrame);
+
</ins><span class="cx"> // Check to see if the underlying media has seeked, which would require us to "flush"
</span><span class="cx"> // our outstanding buffers.
</span><span class="cx"> if (rangeStart != m_endTimeAtLastProcess)
</span><del>- m_readCount = m_writeCount;
</del><ins>+ m_seekTo.store(endFrame);
</ins><span class="cx">
</span><span class="cx"> m_startTimeAtLastProcess = rangeStart;
</span><span class="cx"> m_endTimeAtLastProcess = rangeStart + rangeDuration;
</span><span class="lines">@@ -372,10 +388,9 @@
</span><span class="cx"> // StartOfStream indicates a discontinuity, such as when an AVPlayerItem is re-added
</span><span class="cx"> // to an AVPlayer, so "flush" outstanding buffers.
</span><span class="cx"> if (flagsOut && *flagsOut & kMTAudioProcessingTapFlag_StartOfStream)
</span><del>- m_readCount = m_writeCount;
</del><ins>+ m_seekTo.store(endFrame);
</ins><span class="cx">
</span><del>- m_ringBuffer->store(bufferListInOut, itemCount, m_writeCount);
- m_writeCount += itemCount;
</del><ins>+ m_ringBuffer->store(bufferListInOut, itemCount, endFrame);
</ins><span class="cx">
</span><span class="cx"> // Mute the default audio playback by zeroing the tap-owned buffers.
</span><span class="cx"> for (uint32_t i = 0; i < bufferListInOut->mNumberBuffers; ++i) {
</span></span></pre>
</div>
</div>
</body>
</html>