⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 278307 in webkit


Ignore:
Timestamp:
Jun 1, 2021, 8:09:47 AM (5 years ago)
Author:
Chris Dumez
Message:

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:
Location:
trunk/Source/WebCore
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r278306 r278307  
     12021-06-01  Chris Dumez  <cdumez@apple.com>
     2
     3        Fix thread safety issues in WaveShaperProcessor
     4        https://bugs.webkit.org/show_bug.cgi?id=226478
     5
     6        Reviewed by Youenn Fablet.
     7
     8        Adopt thread safety analysis annotations in WaveShaperProcessor and fix bugs
     9        found by clang. In particular, the following issues were fixed:
     10        - WaveShaperDSPKernel::latencyTime() was failing to grab the lock before accessing
     11          the WaveShaperProcessor's oversample on the rendering thread, even though
     12          oversample gets modified on the main thread.
     13        - WaveShaperNode::propagatesSilence() was failing to grab the lock before accessing
     14          the WaveShaperProcessor's curve on the rendering thread, even though the curve
     15          gets modified on the main thread.
     16
     17        * Modules/webaudio/AudioBasicProcessorNode.h:
     18        (WebCore::AudioBasicProcessorNode::processor const):
     19        * Modules/webaudio/WaveShaperDSPKernel.cpp:
     20        (WebCore::WaveShaperDSPKernel::process):
     21        (WebCore::WaveShaperDSPKernel::processCurve):
     22        (WebCore::WaveShaperDSPKernel::latencyTime const):
     23        * Modules/webaudio/WaveShaperDSPKernel.h:
     24        * Modules/webaudio/WaveShaperNode.cpp:
     25        (WebCore::WaveShaperNode::create):
     26        (WebCore::WaveShaperNode::setCurveForBindings):
     27        (WebCore::WaveShaperNode::curveForBindings):
     28        (WebCore::WaveShaperNode::setOversampleForBindings):
     29        (WebCore::WaveShaperNode::oversampleForBindings const):
     30        (WebCore::WaveShaperNode::propagatesSilence const):
     31        * Modules/webaudio/WaveShaperNode.h:
     32        * Modules/webaudio/WaveShaperNode.idl:
     33        * Modules/webaudio/WaveShaperProcessor.cpp:
     34        (WebCore::WaveShaperProcessor::setCurveForBindings):
     35        (WebCore::WaveShaperProcessor::setOversampleForBindings):
     36        (WebCore::WaveShaperProcessor::process):
     37        * Modules/webaudio/WaveShaperProcessor.h:
     38
    1392021-06-01  Chris Dumez  <cdumez@apple.com>
    240

  • trunk/Source/WebCore/Modules/webaudio/AudioBasicProcessorNode.h

    r267604 r278307  
    5959
    6060    AudioProcessor* processor() { return m_processor.get(); }
     61    const AudioProcessor* processor() const { return m_processor.get(); }
     62
    6163    std::unique_ptr<AudioProcessor> m_processor;
    6264};

  • trunk/Source/WebCore/Modules/webaudio/WaveShaperDSPKernel.cpp

    r267541 r278307  
    6060void WaveShaperDSPKernel::process(const float* source, float* destination, size_t framesToProcess)
    6161{
     62    assertIsHeld(waveShaperProcessor()->processLock());
    6263    switch (waveShaperProcessor()->oversample()) {
    6364    case WaveShaperProcessor::OverSampleNone:
    … …  
    8081    ASSERT(source && destination && waveShaperProcessor());
    8182
     83    assertIsHeld(waveShaperProcessor()->processLock());
    8284    Float32Array* curve = waveShaperProcessor()->curve();
    8385    if (!curve) {
    … …  
    164166double WaveShaperDSPKernel::latencyTime() const
    165167{
     168    if (!waveShaperProcessor()->processLock().tryLock())
     169        return std::numeric_limits<double>::infinity();
     170
     171    Locker locker { AdoptLock, waveShaperProcessor()->processLock() };
     172
    166173    size_t latencyFrames = 0;
    167     WaveShaperDSPKernel* kernel = const_cast<WaveShaperDSPKernel*>(this);
    168 
    169     switch (kernel->waveShaperProcessor()->oversample()) {
     174    switch (waveShaperProcessor()->oversample()) {
    170175    case WaveShaperProcessor::OverSampleNone:
    171176        break;

  • trunk/Source/WebCore/Modules/webaudio/WaveShaperDSPKernel.h

    r266417 r278307  
    4141
    4242    // AudioDSPKernel
    43     void process(const float* source, float* dest, size_t framesToProcess) override;
    44     void reset() override;
    45     double tailTime() const override { return 0; }
    46     double latencyTime() const override;
     43    void process(const float* source, float* dest, size_t framesToProcess) final;
     44    void reset() final;
     45    double tailTime() const final { return 0; }
     46    double latencyTime() const final;
    4747
    4848    // Oversampling requires more resources, so let's only allocate them if needed.
    … …  
    6060
    6161    WaveShaperProcessor* waveShaperProcessor() { return static_cast<WaveShaperProcessor*>(processor()); }
     62    const WaveShaperProcessor* waveShaperProcessor() const { return static_cast<const WaveShaperProcessor*>(processor()); }
    6263
    6364    // Oversampling.

  • trunk/Source/WebCore/Modules/webaudio/WaveShaperNode.cpp

    r277709 r278307  
    5454
    5555    if (curve) {
    56         result = node->setCurve(WTFMove(curve));
     56        result = node->setCurveForBindings(WTFMove(curve));
    5757        if (result.hasException())
    5858            return result.releaseException();
    5959    }
    6060
    61     node->setOversample(options.oversample);
     61    node->setOversampleForBindings(options.oversample);
    6262
    6363    return node;
    … …  
    7272}
    7373
    74 ExceptionOr<void> WaveShaperNode::setCurve(RefPtr<Float32Array>&& curve)
     74ExceptionOr<void> WaveShaperNode::setCurveForBindings(RefPtr<Float32Array>&& curve)
    7575{
    7676    ASSERT(isMainThread());
    … …  
    8686    }
    8787
    88     waveShaperProcessor()->setCurve(curve.get());
     88    waveShaperProcessor()->setCurveForBindings(curve.get());
    8989    return { };
    9090}
    9191
    92 Float32Array* WaveShaperNode::curve()
     92Float32Array* WaveShaperNode::curveForBindings()
    9393{
    94     return waveShaperProcessor()->curve();
     94    ASSERT(isMainThread());
     95    return waveShaperProcessor()->curveForBindings();
    9596}
    9697
    … …  
    109110}
    110111
    111 void WaveShaperNode::setOversample(OverSampleType type)
     112void WaveShaperNode::setOversampleForBindings(OverSampleType type)
    112113{
    113114    ASSERT(isMainThread());
    … …  
    116117    // Synchronize with any graph changes or changes to channel configuration.
    117118    Locker contextLocker { context().graphLock() };
    118     waveShaperProcessor()->setOversample(processorType(type));
     119    waveShaperProcessor()->setOversampleForBindings(processorType(type));
    119120}
    120121
    121 auto WaveShaperNode::oversample() const -> OverSampleType
     122auto WaveShaperNode::oversampleForBindings() const -> OverSampleType
    122123{
    123     switch (const_cast<WaveShaperNode*>(this)->waveShaperProcessor()->oversample()) {
     124    ASSERT(isMainThread());
     125    switch (waveShaperProcessor()->oversampleForBindings()) {
    124126    case WaveShaperProcessor::OverSampleNone:
    125127        return OverSampleType::None;
    … …  
    135137bool WaveShaperNode::propagatesSilence() const
    136138{
    137     auto curve = const_cast<WaveShaperNode*>(this)->curve();
     139    if (!waveShaperProcessor()->processLock().tryLock())
     140        return false;
     141
     142    Locker locker { AdoptLock, waveShaperProcessor()->processLock() };
     143    auto curve = waveShaperProcessor()->curve();
    138144    return !curve || !curve->length();
    139145}

  • trunk/Source/WebCore/Modules/webaudio/WaveShaperNode.h

    r265765 r278307  
    4141
    4242    // setCurve() is called on the main thread.
    43     ExceptionOr<void> setCurve(RefPtr<Float32Array>&&);
    44     Float32Array* curve();
     43    ExceptionOr<void> setCurveForBindings(RefPtr<Float32Array>&&);
     44    Float32Array* curveForBindings();
    4545
    46     void setOversample(OverSampleType);
    47     OverSampleType oversample() const;
     46    void setOversampleForBindings(OverSampleType);
     47    OverSampleType oversampleForBindings() const;
    4848
    4949    double latency() const { return latencyTime(); }
    … …  
    5555
    5656    WaveShaperProcessor* waveShaperProcessor() { return static_cast<WaveShaperProcessor*>(processor()); }
     57    const WaveShaperProcessor* waveShaperProcessor() const { return static_cast<const WaveShaperProcessor*>(processor()); }
    5758};
    5859

  • trunk/Source/WebCore/Modules/webaudio/WaveShaperNode.idl

    r276715 r278307  
    3030    [EnabledBySetting=WebAudio] constructor(BaseAudioContext context, optional WaveShaperOptions options);
    3131
    32     attribute Float32Array? curve;
    33     attribute OverSampleType oversample;
     32    [ImplementedAs=curveForBindings] attribute Float32Array? curve;
     33    [ImplementedAs=oversampleForBindings] attribute OverSampleType oversample;
    3434};

  • trunk/Source/WebCore/Modules/webaudio/WaveShaperProcessor.cpp

    r277932 r278307  
    4949}
    5050
    51 void WaveShaperProcessor::setCurve(Float32Array* curve)
     51void WaveShaperProcessor::setCurveForBindings(Float32Array* curve)
    5252{
     53    ASSERT(isMainThread());
    5354    // This synchronizes with process().
    5455    Locker locker { m_processLock };
    … …  
    5758}
    5859
    59 void WaveShaperProcessor::setOversample(OverSampleType oversample)
     60void WaveShaperProcessor::setOversampleForBindings(OverSampleType oversample)
    6061{
     62    ASSERT(isMainThread());
    6163    // This synchronizes with process().
    6264    Locker locker { m_processLock };
    … …  
    6466    m_oversample = oversample;
    6567
    66     if (oversample != OverSampleNone) {
    67         for (auto& audioDSPKernel : m_kernels) {
    68             WaveShaperDSPKernel& kernel = static_cast<WaveShaperDSPKernel&>(*audioDSPKernel);
    69             kernel.lazyInitializeOversampling();
    70         }
    71     }
     68    if (oversample == OverSampleNone)
     69        return;
     70
     71    for (auto& audioDSPKernel : m_kernels)
     72        static_cast<WaveShaperDSPKernel&>(*audioDSPKernel).lazyInitializeOversampling();
    7273}
    7374
    … …  
    9394
    9495    // For each channel of our input, process using the corresponding WaveShaperDSPKernel into the output channel.
    95     for (unsigned i = 0; i < m_kernels.size(); ++i)
    96         m_kernels[i]->process(source->channel(i)->data(), destination->channel(i)->mutableData(), framesToProcess);
     96    for (size_t i = 0; i < m_kernels.size(); ++i)
     97        static_cast<WaveShaperDSPKernel&>(*m_kernels[i]).process(source->channel(i)->data(), destination->channel(i)->mutableData(), framesToProcess);
    9798}
    9899

  • trunk/Source/WebCore/Modules/webaudio/WaveShaperProcessor.h

    r268001 r278307  
    4949    virtual ~WaveShaperProcessor();
    5050
    51     std::unique_ptr<AudioDSPKernel> createKernel() override;
     51    std::unique_ptr<AudioDSPKernel> createKernel() final;
    5252
    53     void process(const AudioBus* source, AudioBus* destination, size_t framesToProcess) override;
     53    void process(const AudioBus* source, AudioBus* destination, size_t framesToProcess) final;
    5454
    55     void setCurve(Float32Array*);
    56     Float32Array* curve() { return m_curve.get(); }
     55    void setCurveForBindings(Float32Array*);
     56    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.
     57    Float32Array* curve() const WTF_REQUIRES_LOCK(m_processLock) { return m_curve.get(); }
    5758
    58     void setOversample(OverSampleType);
    59     OverSampleType oversample() const { return m_oversample; }
     59    void setOversampleForBindings(OverSampleType);
     60    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.
     61    OverSampleType oversample() const WTF_REQUIRES_LOCK(m_processLock) { return m_oversample; }
     62
     63    Lock& processLock() const WTF_RETURNS_LOCK(m_processLock) { return m_processLock; }
    6064
    6165private:
    6266    // m_curve represents the non-linear shaping curve.
    63     RefPtr<Float32Array> m_curve;
     67    RefPtr<Float32Array> m_curve WTF_GUARDED_BY_LOCK(m_processLock);
    6468
    65     OverSampleType m_oversample { OverSampleNone };
     69    OverSampleType m_oversample WTF_GUARDED_BY_LOCK(m_processLock) { OverSampleNone };
    6670
    6771    // This synchronizes process() with setCurve().
Note: See TracChangeset for help on using the changeset viewer.