Title: [285680] branches/safari-612.3.6.1-branch/Source/WebCore
- Revision
- 285680
- Author
- [email protected]
- Date
- 2021-11-11 15:41:28 -0800 (Thu, 11 Nov 2021)
Log 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
Modified Paths
Diff
Modified: branches/safari-612.3.6.1-branch/Source/WebCore/ChangeLog (285679 => 285680)
--- branches/safari-612.3.6.1-branch/Source/WebCore/ChangeLog 2021-11-11 23:41:25 UTC (rev 285679)
+++ branches/safari-612.3.6.1-branch/Source/WebCore/ChangeLog 2021-11-11 23:41:28 UTC (rev 285680)
@@ -1,5 +1,85 @@
2021-11-11 Alan Coon <[email protected]>
+ 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
+
+ 2021-11-04 Cameron McCormack <[email protected]>
+
+ 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):
+
+2021-11-11 Alan Coon <[email protected]>
+
Cherry-pick r284434. rdar://problem/77969801
WebM with invalid size should fail to load with error
Modified: branches/safari-612.3.6.1-branch/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h (285679 => 285680)
--- branches/safari-612.3.6.1-branch/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h 2021-11-11 23:41:25 UTC (rev 285679)
+++ branches/safari-612.3.6.1-branch/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h 2021-11-11 23:41:28 UTC (rev 285680)
@@ -419,6 +419,7 @@
mutable long long m_cachedTotalBytes { 0 };
unsigned m_pendingStatusChanges { 0 };
int m_cachedItemStatus;
+ int m_runLoopNestingLevel { 0 };
MediaPlayer::BufferingPolicy m_bufferingPolicy { MediaPlayer::BufferingPolicy::Default };
bool m_cachedLikelyToKeepUp { false };
bool m_cachedBufferEmpty { false };
@@ -438,7 +439,6 @@
mutable bool m_allowsWirelessVideoPlayback { true };
bool m_shouldPlayToPlaybackTarget { false };
#endif
- bool m_runningModalPaint { false };
bool m_waitForVideoOutputMediaDataWillChangeTimedOut { false };
bool m_haveBeenAskedToPaint { false };
};
Modified: branches/safari-612.3.6.1-branch/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm (285679 => 285680)
--- branches/safari-612.3.6.1-branch/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm 2021-11-11 23:41:25 UTC (rev 285679)
+++ branches/safari-612.3.6.1-branch/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm 2021-11-11 23:41:28 UTC (rev 285680)
@@ -2563,21 +2563,31 @@
{
if (m_waitForVideoOutputMediaDataWillChangeTimedOut)
return;
- [m_videoOutput requestNotificationOfMediaDataChangeWithAdvanceInterval:0];
// Wait for 1 second.
MonotonicTime start = MonotonicTime::now();
- RunLoop::Timer<MediaPlayerPrivateAVFoundationObjC> timeoutTimer { RunLoop::main(), [] {
- RunLoop::main().stop();
- } };
- timeoutTimer.startOneShot(1_s);
+ std::optional<RunLoop::Timer<MediaPlayerPrivateAVFoundationObjC>> timeoutTimer;
- m_runningModalPaint = true;
+ if (!m_runLoopNestingLevel) {
+ [m_videoOutput requestNotificationOfMediaDataChangeWithAdvanceInterval:0];
+
+ timeoutTimer.emplace(RunLoop::main(), [&] {
+ RunLoop::main().stop();
+ });
+ timeoutTimer->startOneShot(1_s);
+ }
+
+ ++m_runLoopNestingLevel;
RunLoop::run();
- m_runningModalPaint = false;
+ --m_runLoopNestingLevel;
- bool satisfied = timeoutTimer.isActive();
+ if (m_runLoopNestingLevel) {
+ RunLoop::main().stop();
+ return;
+ }
+
+ bool satisfied = timeoutTimer->isActive();
if (!satisfied) {
ERROR_LOG(LOGIDENTIFIER, "timed out");
m_waitForVideoOutputMediaDataWillChangeTimedOut = true;
@@ -2587,7 +2597,7 @@
void MediaPlayerPrivateAVFoundationObjC::outputMediaDataWillChange()
{
- if (m_runningModalPaint)
+ if (m_runLoopNestingLevel)
RunLoop::main().stop();
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
