Changeset 246685 in webkit


Ignore:
Timestamp:
Jun 21, 2019 11:01:13 AM (5 years ago)
Author:
youenn@apple.com
Message:

Safari crashes after ~2028 OfflineAudioContext objects are created (they never get garbage collected, consuming a thread each)
https://bugs.webkit.org/show_bug.cgi?id=198964
<rdar://problem/51891520>

Reviewed by Jer Noble.

Source/WebCore:

Move from setPendingActivity/unsetPendingActivity to an
m_pendingActivity member which is easier to manage.

Keep setting a pending activity for AudioContext at construction time
but do not do that for Offline contexts.
Instead, set the pending activity when startRendering is called.
Unset the pending activity when the rendering activity is finished.

Make m_audioDecoder a unique pointer so that it can lazily be initialized.
This removes the burden of creating an audio decoder thread for each context.

Test: webaudio/offlineaudiocontext-gc.html

  • Modules/webaudio/AudioContext.cpp:

(WebCore::AudioContext::AudioContext):
(WebCore::AudioContext::constructCommon):
(WebCore::AudioContext::clear):
(WebCore::AudioContext::decodeAudioData):
(WebCore::AudioContext::startRendering):
(WebCore::AudioContext::finishedRendering):
(WebCore::AudioContext::dispatchEvent):
(WebCore::AudioContext::clearPendingActivity):
(WebCore::AudioContext::makePendingActivity):
To keep it consistent with setPendingActivity/unsetPendingActivity, we
explicitly ref/unref the AudioContext. We should try to remove this ref/unref.

  • Modules/webaudio/AudioContext.h:
  • Modules/webaudio/OfflineAudioDestinationNode.cpp:

(WebCore::OfflineAudioDestinationNode::startRendering):

LayoutTests:

  • webaudio/offlineaudiocontext-gc-expected.txt: Added.
  • webaudio/offlineaudiocontext-gc.html: Added.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r246683 r246685  
     12019-06-21  Youenn Fablet  <youenn@apple.com>
     2
     3        Safari crashes after ~2028 OfflineAudioContext objects are created (they never get garbage collected, consuming a thread each)
     4        https://bugs.webkit.org/show_bug.cgi?id=198964
     5        <rdar://problem/51891520>
     6
     7        Reviewed by Jer Noble.
     8
     9        * webaudio/offlineaudiocontext-gc-expected.txt: Added.
     10        * webaudio/offlineaudiocontext-gc.html: Added.
     11
    1122019-06-21  Truitt Savell  <tsavell@apple.com>
    213
  • trunk/Source/WebCore/ChangeLog

    r246683 r246685  
     12019-06-21  Youenn Fablet  <youenn@apple.com>
     2
     3        Safari crashes after ~2028 OfflineAudioContext objects are created (they never get garbage collected, consuming a thread each)
     4        https://bugs.webkit.org/show_bug.cgi?id=198964
     5        <rdar://problem/51891520>
     6
     7        Reviewed by Jer Noble.
     8
     9        Move from setPendingActivity/unsetPendingActivity to an
     10        m_pendingActivity member which is easier to manage.
     11
     12        Keep setting a pending activity for AudioContext at construction time
     13        but do not do that for Offline contexts.
     14        Instead, set the pending activity when startRendering is called.
     15        Unset the pending activity when the rendering activity is finished.
     16
     17        Make m_audioDecoder a unique pointer so that it can lazily be initialized.
     18        This removes the burden of creating an audio decoder thread for each context.
     19
     20        Test: webaudio/offlineaudiocontext-gc.html
     21
     22        * Modules/webaudio/AudioContext.cpp:
     23        (WebCore::AudioContext::AudioContext):
     24        (WebCore::AudioContext::constructCommon):
     25        (WebCore::AudioContext::clear):
     26        (WebCore::AudioContext::decodeAudioData):
     27        (WebCore::AudioContext::startRendering):
     28        (WebCore::AudioContext::finishedRendering):
     29        (WebCore::AudioContext::dispatchEvent):
     30        (WebCore::AudioContext::clearPendingActivity):
     31        (WebCore::AudioContext::makePendingActivity):
     32        To keep it consistent with setPendingActivity/unsetPendingActivity, we
     33        explicitly ref/unref the AudioContext. We should try to remove this ref/unref.
     34        * Modules/webaudio/AudioContext.h:
     35        * Modules/webaudio/OfflineAudioDestinationNode.cpp:
     36        (WebCore::OfflineAudioDestinationNode::startRendering):
     37
    1382019-06-21  Truitt Savell  <tsavell@apple.com>
    239
  • trunk/Source/WebCore/Modules/webaudio/AudioContext.cpp

    r246437 r246685  
    100100#include <wtf/Ref.h>
    101101#include <wtf/RefCounted.h>
     102#include <wtf/Scope.h>
    102103#include <wtf/text/WTFString.h>
    103104
     
    142143    , m_eventQueue(std::make_unique<GenericEventQueue>(*this))
    143144{
     145    // According to spec AudioContext must die only after page navigate.
     146    // Lets mark it as ActiveDOMObject with pending activity and unmark it in clear method.
     147    makePendingActivity();
     148
    144149    constructCommon();
    145150
     
    173178void AudioContext::constructCommon()
    174179{
    175     // According to spec AudioContext must die only after page navigate.
    176     // Lets mark it as ActiveDOMObject with pending activity and unmark it in clear method.
    177     setPendingActivity(*this);
    178 
    179180    FFTFrame::initialize();
    180181   
     
    243244void AudioContext::clear()
    244245{
     246    Ref<AudioContext> protectedThis(*this);
     247
    245248    // We have to release our reference to the destination node before the context will ever be deleted since the destination node holds a reference to the context.
    246249    if (m_destinationNode)
     
    254257    } while (m_nodesToDelete.size());
    255258
    256     // It was set in constructCommon.
    257     unsetPendingActivity(*this);
     259    clearPendingActivity();
    258260}
    259261
     
    430432void AudioContext::decodeAudioData(Ref<ArrayBuffer>&& audioData, RefPtr<AudioBufferCallback>&& successCallback, RefPtr<AudioBufferCallback>&& errorCallback)
    431433{
    432     m_audioDecoder.decodeAsync(WTFMove(audioData), sampleRate(), WTFMove(successCallback), WTFMove(errorCallback));
     434    if (!m_audioDecoder)
     435        m_audioDecoder = std::make_unique<AsyncAudioDecoder>();
     436    m_audioDecoder->decodeAsync(WTFMove(audioData), sampleRate(), WTFMove(successCallback), WTFMove(errorCallback));
    433437}
    434438
     
    11421146        return;
    11431147
     1148    makePendingActivity();
     1149
    11441150    destination()->startRendering();
    11451151    setState(State::Running);
     
    11771183}
    11781184
    1179 void AudioContext::fireCompletionEvent()
    1180 {
     1185void AudioContext::finishedRendering(bool didRendering)
     1186{
     1187    ASSERT(isOfflineContext());
    11811188    ASSERT(isMainThread());
    11821189    if (!isMainThread())
    11831190        return;
    11841191
    1185     ALWAYS_LOG(LOGIDENTIFIER);
    1186    
     1192    auto clearPendingActivityIfExitEarly = WTF::makeScopeExit([this] {
     1193        clearPendingActivity();
     1194    });
     1195
     1196
     1197    ALWAYS_LOG(LOGIDENTIFIER);
     1198
     1199    if (!didRendering)
     1200        return;
     1201
    11871202    AudioBuffer* renderedBuffer = m_renderTarget.get();
    11881203    setState(State::Closed);
     
    11931208
    11941209    // Avoid firing the event if the document has already gone away.
    1195     if (!m_isStopScheduled) {
    1196         // Call the offline rendering completion event listener.
    1197         m_eventQueue->enqueueEvent(OfflineAudioCompletionEvent::create(renderedBuffer));
    1198     }
     1210    if (m_isStopScheduled)
     1211        return;
     1212
     1213    clearPendingActivityIfExitEarly.release();
     1214    m_eventQueue->enqueueEvent(OfflineAudioCompletionEvent::create(renderedBuffer));
     1215}
     1216
     1217void AudioContext::dispatchEvent(Event& event)
     1218{
     1219    EventTarget::dispatchEvent(event);
     1220    if (event.eventInterface() == OfflineAudioCompletionEventInterfaceType)
     1221        clearPendingActivity();
    11991222}
    12001223
     
    13481371}
    13491372
     1373void AudioContext::clearPendingActivity()
     1374{
     1375    if (!m_pendingActivity)
     1376        return;
     1377    m_pendingActivity = nullptr;
     1378    // FIXME: Remove this specific deref() and ref() call in makePendingActivity().
     1379    deref();
     1380}
     1381
     1382void AudioContext::makePendingActivity()
     1383{
     1384    if (m_pendingActivity)
     1385        return;
     1386    m_pendingActivity = ActiveDOMObject::makePendingActivity(*this);
     1387    ref();
     1388}
     1389
    13501390#if !RELEASE_LOG_DISABLED
    13511391WTFLogChannel& AudioContext::logChannel() const
  • trunk/Source/WebCore/Modules/webaudio/AudioContext.h

    r246490 r246685  
    260260
    261261    void startRendering();
    262     void fireCompletionEvent();
    263    
     262    void finishedRendering(bool didRendering);
     263
    264264    static unsigned s_hardwareContextCount;
    265265
     
    298298   
    299299    static bool isSampleRateRangeGood(float sampleRate);
    300    
     300    void clearPendingActivity();
     301    void makePendingActivity();
     302
    301303private:
    302304    void constructCommon();
     
    321323    // EventTarget
    322324    ScriptExecutionContext* scriptExecutionContext() const final;
     325    void dispatchEvent(Event&) final;
    323326
    324327    // MediaProducer
     
    430433    Thread* volatile m_graphOwnerThread { nullptr }; // if the lock is held then this is the thread which owns it, otherwise == nullptr.
    431434
    432     AsyncAudioDecoder m_audioDecoder;
     435    std::unique_ptr<AsyncAudioDecoder> m_audioDecoder;
    433436
    434437    // This is considering 32 is large enough for multiple channels audio.
     
    442445
    443446    State m_state { State::Suspended };
     447    RefPtr<PendingActivity<AudioContext>> m_pendingActivity;
    444448};
    445449
  • trunk/Source/WebCore/Modules/webaudio/OfflineAudioDestinationNode.cpp

    r244145 r246685  
    9595        bool didRender = offlineRender();
    9696        callOnMainThread([this, didRender] {
    97             if (didRender)
    98                 context().fireCompletionEvent();
     97            context().finishedRendering(didRender);
    9998            deref();
    10099        });
Note: See TracChangeset for help on using the changeset viewer.