Bug 232695 - nested run loops under MediaPlayerPrivateAVFoundationObjC::waitForVideoOutputMediaDataWillChange can cause hang when timeout fires
Summary: nested run loops under MediaPlayerPrivateAVFoundationObjC::waitForVideoOutput...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Cameron McCormack (:heycam)
URL:
Keywords: InRadar
Depends on:
Blocks: 230766
  Show dependency tree / graph
 
Reported: 2021-11-03 21:24 PDT by Cameron McCormack (:heycam)
Modified: 2021-11-04 20:28 PDT (History)
7 users (show)

See Also:

Attachments
Patch (5.76 KB, patch)
2021-11-03 21:32 PDT, Cameron McCormack (:heycam) 		no flags Details | Formatted Diff | Diff
Patch for landing (5.73 KB, patch)
2021-11-04 16:19 PDT, Cameron McCormack (:heycam) 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Cameron McCormack (:heycam) 2021-11-03 21:24:34 PDT 
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.

I encountered this with my bug 232565 patch applied running webgl/1.0.x/conformance/textures/misc/texture-corner-case-videos.html.
Comment 1 Radar WebKit Bug Importer 2021-11-03 21:25:30 PDT 
<rdar://problem/85004449>
Comment 2 Cameron McCormack (:heycam) 2021-11-03 21:32:27 PDT 
(In reply to Cameron McCormack (:heycam) from comment #0)
> I encountered this with my bug 232565 patch applied running
> webgl/1.0.x/conformance/textures/misc/texture-corner-case-videos.html.

bug 230766 rather
Comment 3 Cameron McCormack (:heycam) 2021-11-03 21:32:55 PDT 
Created attachment 443274 [details]
Patch
Comment 4 EWS 2021-11-04 13:57:10 PDT 
Tools/Scripts/svn-apply failed to apply attachment 443274 [details] to trunk.
Please resolve the conflicts and upload a new patch.
Comment 5 Cameron McCormack (:heycam) 2021-11-04 16:19:08 PDT 
Created attachment 443353 [details]
Patch for landing
Comment 6 Cameron McCormack (:heycam) 2021-11-04 16:21:55 PDT 
The patch conflicted with bug 232676, which made me realize it was wrong to be modifying m_waitForVideoOutputMediaDataWillChangeTimedOut inside the RunLoop::main() task, since in WebKitLegacy that's the UI thread, and the MediaPlayerPrivateAVFoundationObjC object is created on the web thread.  So I've tweaked it to avoid that.
Comment 7 EWS 2021-11-04 20:28:01 PDT 
Committed r285330 (243895@main): <https://commits.webkit.org/243895@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 443353 [details].