Diff
Modified: trunk/Source/WebCore/ChangeLog (278306 => 278307)
--- trunk/Source/WebCore/ChangeLog 2021-06-01 14:56:04 UTC (rev 278306)
+++ trunk/Source/WebCore/ChangeLog 2021-06-01 15:09:47 UTC (rev 278307)
@@ -1,5 +1,43 @@
2021-06-01 Chris Dumez <cdu...@apple.com>
+ Fix thread safety issues in WaveShaperProcessor
+ https://bugs.webkit.org/show_bug.cgi?id=226478
+
+ Reviewed by Youenn Fablet.
+
+ Adopt thread safety analysis annotations in WaveShaperProcessor and fix bugs
+ found by clang. In particular, the following issues were fixed:
+ - WaveShaperDSPKernel::latencyTime() was failing to grab the lock before accessing
+ the WaveShaperProcessor's oversample on the rendering thread, even though
+ oversample gets modified on the main thread.
+ - WaveShaperNode::propagatesSilence() was failing to grab the lock before accessing
+ the WaveShaperProcessor's curve on the rendering thread, even though the curve
+ gets modified on the main thread.
+
+ * Modules/webaudio/AudioBasicProcessorNode.h:
+ (WebCore::AudioBasicProcessorNode::processor const):
+ * Modules/webaudio/WaveShaperDSPKernel.cpp:
+ (WebCore::WaveShaperDSPKernel::process):
+ (WebCore::WaveShaperDSPKernel::processCurve):
+ (WebCore::WaveShaperDSPKernel::latencyTime const):
+ * Modules/webaudio/WaveShaperDSPKernel.h:
+ * Modules/webaudio/WaveShaperNode.cpp:
+ (WebCore::WaveShaperNode::create):
+ (WebCore::WaveShaperNode::setCurveForBindings):
+ (WebCore::WaveShaperNode::curveForBindings):
+ (WebCore::WaveShaperNode::setOversampleForBindings):
+ (WebCore::WaveShaperNode::oversampleForBindings const):
+ (WebCore::WaveShaperNode::propagatesSilence const):
+ * Modules/webaudio/WaveShaperNode.h:
+ * Modules/webaudio/WaveShaperNode.idl:
+ * Modules/webaudio/WaveShaperProcessor.cpp:
+ (WebCore::WaveShaperProcessor::setCurveForBindings):
+ (WebCore::WaveShaperProcessor::setOversampleForBindings):
+ (WebCore::WaveShaperProcessor::process):
+ * Modules/webaudio/WaveShaperProcessor.h:
+
+2021-06-01 Chris Dumez <cdu...@apple.com>
+
Fix thread safety issues in MediaElementAudioSourceNode
https://bugs.webkit.org/show_bug.cgi?id=226475
Modified: trunk/Source/WebCore/Modules/webaudio/AudioBasicProcessorNode.h (278306 => 278307)
--- trunk/Source/WebCore/Modules/webaudio/AudioBasicProcessorNode.h 2021-06-01 14:56:04 UTC (rev 278306)
+++ trunk/Source/WebCore/Modules/webaudio/AudioBasicProcessorNode.h 2021-06-01 15:09:47 UTC (rev 278307)
@@ -58,6 +58,8 @@
bool requiresTailProcessing() const override;
AudioProcessor* processor() { return m_processor.get(); }
+ const AudioProcessor* processor() const { return m_processor.get(); }
+
std::unique_ptr<AudioProcessor> m_processor;
};
Modified: trunk/Source/WebCore/Modules/webaudio/WaveShaperDSPKernel.cpp (278306 => 278307)
--- trunk/Source/WebCore/Modules/webaudio/WaveShaperDSPKernel.cpp 2021-06-01 14:56:04 UTC (rev 278306)
+++ trunk/Source/WebCore/Modules/webaudio/WaveShaperDSPKernel.cpp 2021-06-01 15:09:47 UTC (rev 278307)
@@ -59,6 +59,7 @@
void WaveShaperDSPKernel::process(const float* source, float* destination, size_t framesToProcess)
{
+ assertIsHeld(waveShaperProcessor()->processLock());
switch (waveShaperProcessor()->oversample()) {
case WaveShaperProcessor::OverSampleNone:
processCurve(source, destination, framesToProcess);
@@ -79,6 +80,7 @@
{
ASSERT(source && destination && waveShaperProcessor());
+ assertIsHeld(waveShaperProcessor()->processLock());
Float32Array* curve = waveShaperProcessor()->curve();
if (!curve) {
// Act as "straight wire" pass-through if no curve is set.
@@ -163,10 +165,13 @@
double WaveShaperDSPKernel::latencyTime() const
{
+ if (!waveShaperProcessor()->processLock().tryLock())
+ return std::numeric_limits<double>::infinity();
+
+ Locker locker { AdoptLock, waveShaperProcessor()->processLock() };
+
size_t latencyFrames = 0;
- WaveShaperDSPKernel* kernel = const_cast<WaveShaperDSPKernel*>(this);
-
- switch (kernel->waveShaperProcessor()->oversample()) {
+ switch (waveShaperProcessor()->oversample()) {
case WaveShaperProcessor::OverSampleNone:
break;
case WaveShaperProcessor::OverSample2x:
Modified: trunk/Source/WebCore/Modules/webaudio/WaveShaperDSPKernel.h (278306 => 278307)
--- trunk/Source/WebCore/Modules/webaudio/WaveShaperDSPKernel.h 2021-06-01 14:56:04 UTC (rev 278306)
+++ trunk/Source/WebCore/Modules/webaudio/WaveShaperDSPKernel.h 2021-06-01 15:09:47 UTC (rev 278307)
@@ -40,10 +40,10 @@
explicit WaveShaperDSPKernel(WaveShaperProcessor*);
// AudioDSPKernel
- void process(const float* source, float* dest, size_t framesToProcess) override;
- void reset() override;
- double tailTime() const override { return 0; }
- double latencyTime() const override;
+ void process(const float* source, float* dest, size_t framesToProcess) final;
+ void reset() final;
+ double tailTime() const final { return 0; }
+ double latencyTime() const final;
// Oversampling requires more resources, so let's only allocate them if needed.
void lazyInitializeOversampling();
@@ -59,6 +59,7 @@
bool requiresTailProcessing() const final;
WaveShaperProcessor* waveShaperProcessor() { return static_cast<WaveShaperProcessor*>(processor()); }
+ const WaveShaperProcessor* waveShaperProcessor() const { return static_cast<const WaveShaperProcessor*>(processor()); }
// Oversampling.
std::unique_ptr<AudioFloatArray> m_tempBuffer;
Modified: trunk/Source/WebCore/Modules/webaudio/WaveShaperNode.cpp (278306 => 278307)
--- trunk/Source/WebCore/Modules/webaudio/WaveShaperNode.cpp 2021-06-01 14:56:04 UTC (rev 278306)
+++ trunk/Source/WebCore/Modules/webaudio/WaveShaperNode.cpp 2021-06-01 15:09:47 UTC (rev 278307)
@@ -53,12 +53,12 @@
return result.releaseException();
if (curve) {
- result = node->setCurve(WTFMove(curve));
+ result = node->setCurveForBindings(WTFMove(curve));
if (result.hasException())
return result.releaseException();
}
- node->setOversample(options.oversample);
+ node->setOversampleForBindings(options.oversample);
return node;
}
@@ -71,7 +71,7 @@
initialize();
}
-ExceptionOr<void> WaveShaperNode::setCurve(RefPtr<Float32Array>&& curve)
+ExceptionOr<void> WaveShaperNode::setCurveForBindings(RefPtr<Float32Array>&& curve)
{
ASSERT(isMainThread());
DEBUG_LOG(LOGIDENTIFIER);
@@ -85,13 +85,14 @@
curve = WTFMove(clonedCurve);
}
- waveShaperProcessor()->setCurve(curve.get());
+ waveShaperProcessor()->setCurveForBindings(curve.get());
return { };
}
-Float32Array* WaveShaperNode::curve()
+Float32Array* WaveShaperNode::curveForBindings()
{
- return waveShaperProcessor()->curve();
+ ASSERT(isMainThread());
+ return waveShaperProcessor()->curveForBindings();
}
static inline WaveShaperProcessor::OverSampleType processorType(OverSampleType type)
@@ -108,7 +109,7 @@
return WaveShaperProcessor::OverSampleNone;
}
-void WaveShaperNode::setOversample(OverSampleType type)
+void WaveShaperNode::setOversampleForBindings(OverSampleType type)
{
ASSERT(isMainThread());
INFO_LOG(LOGIDENTIFIER, type);
@@ -115,12 +116,13 @@
// Synchronize with any graph changes or changes to channel configuration.
Locker contextLocker { context().graphLock() };
- waveShaperProcessor()->setOversample(processorType(type));
+ waveShaperProcessor()->setOversampleForBindings(processorType(type));
}
-auto WaveShaperNode::oversample() const -> OverSampleType
+auto WaveShaperNode::oversampleForBindings() const -> OverSampleType
{
- switch (const_cast<WaveShaperNode*>(this)->waveShaperProcessor()->oversample()) {
+ ASSERT(isMainThread());
+ switch (waveShaperProcessor()->oversampleForBindings()) {
case WaveShaperProcessor::OverSampleNone:
return OverSampleType::None;
case WaveShaperProcessor::OverSample2x:
@@ -134,7 +136,11 @@
bool WaveShaperNode::propagatesSilence() const
{
- auto curve = const_cast<WaveShaperNode*>(this)->curve();
+ if (!waveShaperProcessor()->processLock().tryLock())
+ return false;
+
+ Locker locker { AdoptLock, waveShaperProcessor()->processLock() };
+ auto curve = waveShaperProcessor()->curve();
return !curve || !curve->length();
}
Modified: trunk/Source/WebCore/Modules/webaudio/WaveShaperNode.h (278306 => 278307)
--- trunk/Source/WebCore/Modules/webaudio/WaveShaperNode.h 2021-06-01 14:56:04 UTC (rev 278306)
+++ trunk/Source/WebCore/Modules/webaudio/WaveShaperNode.h 2021-06-01 15:09:47 UTC (rev 278307)
@@ -40,11 +40,11 @@
static ExceptionOr<Ref<WaveShaperNode>> create(BaseAudioContext&, const WaveShaperOptions& = { });
// setCurve() is called on the main thread.
- ExceptionOr<void> setCurve(RefPtr<Float32Array>&&);
- Float32Array* curve();
+ ExceptionOr<void> setCurveForBindings(RefPtr<Float32Array>&&);
+ Float32Array* curveForBindings();
- void setOversample(OverSampleType);
- OverSampleType oversample() const;
+ void setOversampleForBindings(OverSampleType);
+ OverSampleType oversampleForBindings() const;
double latency() const { return latencyTime(); }
@@ -54,6 +54,7 @@
bool propagatesSilence() const final;
WaveShaperProcessor* waveShaperProcessor() { return static_cast<WaveShaperProcessor*>(processor()); }
+ const WaveShaperProcessor* waveShaperProcessor() const { return static_cast<const WaveShaperProcessor*>(processor()); }
};
String convertEnumerationToString(WebCore::OverSampleType); // in JSOverSampleType.cpp
Modified: trunk/Source/WebCore/Modules/webaudio/WaveShaperNode.idl (278306 => 278307)
--- trunk/Source/WebCore/Modules/webaudio/WaveShaperNode.idl 2021-06-01 14:56:04 UTC (rev 278306)
+++ trunk/Source/WebCore/Modules/webaudio/WaveShaperNode.idl 2021-06-01 15:09:47 UTC (rev 278307)
@@ -29,6 +29,6 @@
] interface WaveShaperNode : AudioNode {
[EnabledBySetting=WebAudio] constructor(BaseAudioContext context, optional WaveShaperOptions options);
- attribute Float32Array? curve;
- attribute OverSampleType oversample;
+ [ImplementedAs=curveForBindings] attribute Float32Array? curve;
+ [ImplementedAs=oversampleForBindings] attribute OverSampleType oversample;
};
Modified: trunk/Source/WebCore/Modules/webaudio/WaveShaperProcessor.cpp (278306 => 278307)
--- trunk/Source/WebCore/Modules/webaudio/WaveShaperProcessor.cpp 2021-06-01 14:56:04 UTC (rev 278306)
+++ trunk/Source/WebCore/Modules/webaudio/WaveShaperProcessor.cpp 2021-06-01 15:09:47 UTC (rev 278307)
@@ -48,8 +48,9 @@
return makeUnique<WaveShaperDSPKernel>(this);
}
-void WaveShaperProcessor::setCurve(Float32Array* curve)
+void WaveShaperProcessor::setCurveForBindings(Float32Array* curve)
{
+ ASSERT(isMainThread());
// This synchronizes with process().
Locker locker { m_processLock };
@@ -56,19 +57,19 @@
m_curve = curve;
}
-void WaveShaperProcessor::setOversample(OverSampleType oversample)
+void WaveShaperProcessor::setOversampleForBindings(OverSampleType oversample)
{
+ ASSERT(isMainThread());
// This synchronizes with process().
Locker locker { m_processLock };
m_oversample = oversample;
- if (oversample != OverSampleNone) {
- for (auto& audioDSPKernel : m_kernels) {
- WaveShaperDSPKernel& kernel = static_cast<WaveShaperDSPKernel&>(*audioDSPKernel);
- kernel.lazyInitializeOversampling();
- }
- }
+ if (oversample == OverSampleNone)
+ return;
+
+ for (auto& audioDSPKernel : m_kernels)
+ static_cast<WaveShaperDSPKernel&>(*audioDSPKernel).lazyInitializeOversampling();
}
void WaveShaperProcessor::process(const AudioBus* source, AudioBus* destination, size_t framesToProcess)
@@ -92,8 +93,8 @@
Locker locker { AdoptLock, m_processLock };
// For each channel of our input, process using the corresponding WaveShaperDSPKernel into the output channel.
- for (unsigned i = 0; i < m_kernels.size(); ++i)
- m_kernels[i]->process(source->channel(i)->data(), destination->channel(i)->mutableData(), framesToProcess);
+ for (size_t i = 0; i < m_kernels.size(); ++i)
+ static_cast<WaveShaperDSPKernel&>(*m_kernels[i]).process(source->channel(i)->data(), destination->channel(i)->mutableData(), framesToProcess);
}
} // namespace WebCore
Modified: trunk/Source/WebCore/Modules/webaudio/WaveShaperProcessor.h (278306 => 278307)
--- trunk/Source/WebCore/Modules/webaudio/WaveShaperProcessor.h 2021-06-01 14:56:04 UTC (rev 278306)
+++ trunk/Source/WebCore/Modules/webaudio/WaveShaperProcessor.h 2021-06-01 15:09:47 UTC (rev 278307)
@@ -48,21 +48,25 @@
virtual ~WaveShaperProcessor();
- std::unique_ptr<AudioDSPKernel> createKernel() override;
+ std::unique_ptr<AudioDSPKernel> createKernel() final;
- void process(const AudioBus* source, AudioBus* destination, size_t framesToProcess) override;
+ void process(const AudioBus* source, AudioBus* destination, size_t framesToProcess) final;
- void setCurve(Float32Array*);
- Float32Array* curve() { return m_curve.get(); }
+ void setCurveForBindings(Float32Array*);
+ Float32Array* curveForBindings() WTF_IGNORES_THREAD_SAFETY_ANALYSIS { ASSERT(isMainThread()); return m_curve.get(); } // Doesn't grab the lock, only safe to call on the main thread.
+ Float32Array* curve() const WTF_REQUIRES_LOCK(m_processLock) { return m_curve.get(); }
- void setOversample(OverSampleType);
- OverSampleType oversample() const { return m_oversample; }
+ void setOversampleForBindings(OverSampleType);
+ OverSampleType oversampleForBindings() const WTF_IGNORES_THREAD_SAFETY_ANALYSIS { ASSERT(isMainThread()); return m_oversample; } // Doesn't grab the lock, only safe to call on the main thread.
+ OverSampleType oversample() const WTF_REQUIRES_LOCK(m_processLock) { return m_oversample; }
+ Lock& processLock() const WTF_RETURNS_LOCK(m_processLock) { return m_processLock; }
+
private:
// m_curve represents the non-linear shaping curve.
- RefPtr<Float32Array> m_curve;
+ RefPtr<Float32Array> m_curve WTF_GUARDED_BY_LOCK(m_processLock);
- OverSampleType m_oversample { OverSampleNone };
+ OverSampleType m_oversample WTF_GUARDED_BY_LOCK(m_processLock) { OverSampleNone };
// This synchronizes process() with setCurve().
mutable Lock m_processLock;
