Title: [285330] trunk/Source/WebCore
- Revision
- 285330
- Author
- [email protected]
- Date
- 2021-11-04 20:27:54 -0700 (Thu, 04 Nov 2021)
Log Message
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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (285329 => 285330)
--- trunk/Source/WebCore/ChangeLog 2021-11-05 03:25:23 UTC (rev 285329)
+++ trunk/Source/WebCore/ChangeLog 2021-11-05 03:27:54 UTC (rev 285330)
@@ -1,3 +1,41 @@
+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-04 Alan Bujtas <[email protected]>
[LFC][IFC] Use objectReplacementCharacter to mark atomic inline box position.
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h (285329 => 285330)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h 2021-11-05 03:25:23 UTC (rev 285329)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h 2021-11-05 03:27:54 UTC (rev 285330)
@@ -428,6 +428,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 };
@@ -447,7 +448,6 @@
mutable bool m_allowsWirelessVideoPlayback { true };
bool m_shouldPlayToPlaybackTarget { false };
#endif
- bool m_runningModalPaint { false };
bool m_haveProcessedChapterTracks { false };
bool m_waitForVideoOutputMediaDataWillChangeTimedOut { false };
bool m_haveBeenAskedToPaint { false };
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm (285329 => 285330)
--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm 2021-11-05 03:25:23 UTC (rev 285329)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm 2021-11-05 03:27:54 UTC (rev 285330)
@@ -2609,21 +2609,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;
@@ -2633,7 +2643,7 @@
void MediaPlayerPrivateAVFoundationObjC::outputMediaDataWillChange()
{
- if (m_runningModalPaint) {
+ if (m_runLoopNestingLevel) {
if (RunLoop::isMain())
RunLoop::main().stop();
else {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
