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

Changeset 285680 in webkit


Ignore:
Timestamp:
Nov 11, 2021, 3:41:28 PM (5 years ago)
Author:
Alan Coon
Message:

Cherry-pick r285330. rdar://problem/85004449

Nested run loops under MediaPlayerPrivateAVFoundationObjC::waitForVideoOutputMediaDataWillChange can cause hang when timeout fires
​https://bugs.webkit.org/show_bug.cgi?id=232695
<rdar://problem/85004449>

Reviewed by Jer Noble.

It's possible for MediaPlayerPrivateAVFoundationObjC::waitForVideoOutputMediaDataWillChange
to be called re-entrantly, if the RunLoop::run call ends up processing
an event that also wants to synchronously update the media image. This
can cause a hang:

  1. Enter the outer waitForVideoOutputMediaDataWillChange call.
  2. Set up the outer timeout timer.
  3. Call RunLoop::run.

3.1. Enter the inner waitForVideoOutputMediaDataWillChange call.
3.2. Set up the inner timeout timer.
3.3. Call RunLoop::run.

3.3.1. Wait for new RunLoop events, and none arrive.
3.3.2. The outer timeout timer fires, calling RunLoop::stop.

3.4. Return from waitForVideoOutputMediaDataWillChange, cancelling

the inner timeout timer.

3.5. Wait for more events on the run loop, forever.

To avoid this, we can set up a single timeout timer, and track the
nesting level of our RunLoop::run calls. The innermost RunLoop::run call
will finish either by the timer firing (which calls RunLoop::stop) or by
the video data updating (which also calls RunLoop::stop, under
outputMediaDataWillChange). Either way, once the innermost
RunLoop::run call is finished, we know we can stop processing all of
the ancestor RunLoop:run calls.

  • platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
  • platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm: (WebCore::MediaPlayerPrivateAVFoundationObjC::waitForVideoOutputMediaDataWillChange): (WebCore::MediaPlayerPrivateAVFoundationObjC::outputMediaDataWillChange):

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@285330 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-612.3.6.1-branch/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-612.3.6.1-branch/Source/WebCore/ChangeLog

    r285679 r285680  
     12021-11-11  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r285330. rdar://problem/85004449
     4
     5    Nested run loops under MediaPlayerPrivateAVFoundationObjC::waitForVideoOutputMediaDataWillChange can cause hang when timeout fires
     6    https://bugs.webkit.org/show_bug.cgi?id=232695
     7    <rdar://problem/85004449>
     8   
     9    Reviewed by Jer Noble.
     10   
     11    It's possible for MediaPlayerPrivateAVFoundationObjC::waitForVideoOutputMediaDataWillChange
     12    to be called re-entrantly, if the RunLoop::run call ends up processing
     13    an event that also wants to synchronously update the media image. This
     14    can cause a hang:
     15   
     16    1. Enter the outer waitForVideoOutputMediaDataWillChange call.
     17    2. Set up the outer timeout timer.
     18    3. Call RunLoop::run.
     19        3.1. Enter the inner waitForVideoOutputMediaDataWillChange call.
     20        3.2. Set up the inner timeout timer.
     21        3.3. Call RunLoop::run.
     22            3.3.1. Wait for new RunLoop events, and none arrive.
     23            3.3.2. The outer timeout timer fires, calling RunLoop::stop.
     24        3.4. Return from waitForVideoOutputMediaDataWillChange, cancelling
     25             the inner timeout timer.
     26        3.5. Wait for more events on the run loop, forever.
     27   
     28    To avoid this, we can set up a single timeout timer, and track the
     29    nesting level of our RunLoop::run calls. The innermost RunLoop::run call
     30    will finish either by the timer firing (which calls RunLoop::stop) or by
     31    the video data updating (which also calls RunLoop::stop, under
     32    outputMediaDataWillChange). Either way, once the innermost
     33    RunLoop::run call is finished, we know we can stop processing all of
     34    the ancestor RunLoop:run calls.
     35   
     36    * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
     37    * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
     38    (WebCore::MediaPlayerPrivateAVFoundationObjC::waitForVideoOutputMediaDataWillChange):
     39    (WebCore::MediaPlayerPrivateAVFoundationObjC::outputMediaDataWillChange):
     40   
     41    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@285330 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     42
     43    2021-11-04  Cameron McCormack  <heycam@apple.com>
     44
     45            Nested run loops under MediaPlayerPrivateAVFoundationObjC::waitForVideoOutputMediaDataWillChange can cause hang when timeout fires
     46            https://bugs.webkit.org/show_bug.cgi?id=232695
     47            <rdar://problem/85004449>
     48
     49            Reviewed by Jer Noble.
     50
     51            It's possible for MediaPlayerPrivateAVFoundationObjC::waitForVideoOutputMediaDataWillChange
     52            to be called re-entrantly, if the RunLoop::run call ends up processing
     53            an event that also wants to synchronously update the media image. This
     54            can cause a hang:
     55
     56            1. Enter the outer waitForVideoOutputMediaDataWillChange call.
     57            2. Set up the outer timeout timer.
     58            3. Call RunLoop::run.
     59                3.1. Enter the inner waitForVideoOutputMediaDataWillChange call.
     60                3.2. Set up the inner timeout timer.
     61                3.3. Call RunLoop::run.
     62                    3.3.1. Wait for new RunLoop events, and none arrive.
     63                    3.3.2. The outer timeout timer fires, calling RunLoop::stop.
     64                3.4. Return from waitForVideoOutputMediaDataWillChange, cancelling
     65                     the inner timeout timer.
     66                3.5. Wait for more events on the run loop, forever.
     67
     68            To avoid this, we can set up a single timeout timer, and track the
     69            nesting level of our RunLoop::run calls. The innermost RunLoop::run call
     70            will finish either by the timer firing (which calls RunLoop::stop) or by
     71            the video data updating (which also calls RunLoop::stop, under
     72            outputMediaDataWillChange). Either way, once the innermost
     73            RunLoop::run call is finished, we know we can stop processing all of
     74            the ancestor RunLoop:run calls.
     75
     76            * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
     77            * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
     78            (WebCore::MediaPlayerPrivateAVFoundationObjC::waitForVideoOutputMediaDataWillChange):
     79            (WebCore::MediaPlayerPrivateAVFoundationObjC::outputMediaDataWillChange):
     80
    1812021-11-11  Alan Coon  <alancoon@apple.com>
    282

  • branches/safari-612.3.6.1-branch/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h

    r285517 r285680  
    420420    unsigned m_pendingStatusChanges { 0 };
    421421    int m_cachedItemStatus;
     422    int m_runLoopNestingLevel { 0 };
    422423    MediaPlayer::BufferingPolicy m_bufferingPolicy { MediaPlayer::BufferingPolicy::Default };
    423424    bool m_cachedLikelyToKeepUp { false };
    … …  
    439440    bool m_shouldPlayToPlaybackTarget { false };
    440441#endif
    441     bool m_runningModalPaint { false };
    442442    bool m_waitForVideoOutputMediaDataWillChangeTimedOut { false };
    443443    bool m_haveBeenAskedToPaint { false };

  • branches/safari-612.3.6.1-branch/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm

    r285517 r285680  
    25642564    if (m_waitForVideoOutputMediaDataWillChangeTimedOut)
    25652565        return;
    2566     [m_videoOutput requestNotificationOfMediaDataChangeWithAdvanceInterval:0];
    25672566
    25682567    // Wait for 1 second.
    25692568    MonotonicTime start = MonotonicTime::now();
    25702569
    2571     RunLoop::Timer<MediaPlayerPrivateAVFoundationObjC> timeoutTimer { RunLoop::main(), [] {
     2570    std::optional<RunLoop::Timer<MediaPlayerPrivateAVFoundationObjC>> timeoutTimer;
     2571
     2572    if (!m_runLoopNestingLevel) {
     2573        [m_videoOutput requestNotificationOfMediaDataChangeWithAdvanceInterval:0];
     2574
     2575        timeoutTimer.emplace(RunLoop::main(), [&] {
     2576            RunLoop::main().stop();
     2577        });
     2578        timeoutTimer->startOneShot(1_s);
     2579    }
     2580
     2581    ++m_runLoopNestingLevel;
     2582    RunLoop::run();
     2583    --m_runLoopNestingLevel;
     2584
     2585    if (m_runLoopNestingLevel) {
    25722586        RunLoop::main().stop();
    2573     } };
    2574     timeoutTimer.startOneShot(1_s);
    2575 
    2576     m_runningModalPaint = true;
    2577     RunLoop::run();
    2578     m_runningModalPaint = false;
    2579 
    2580     bool satisfied = timeoutTimer.isActive();
     2587        return;
     2588    }
     2589
     2590    bool satisfied = timeoutTimer->isActive();
    25812591    if (!satisfied) {
    25822592        ERROR_LOG(LOGIDENTIFIER, "timed out");
    … …  
    25882598void MediaPlayerPrivateAVFoundationObjC::outputMediaDataWillChange()
    25892599{
    2590     if (m_runningModalPaint)
     2600    if (m_runLoopNestingLevel)
    25912601        RunLoop::main().stop();
    25922602}
Note: See TracChangeset for help on using the changeset viewer.