RESOLVED FIXED 63681
Chromium bug: Compositing on a lost context causes latch deadlocks
https://bugs.webkit.org/show_bug.cgi?id=63681
Summary Chromium bug: Compositing on a lost context causes latch deadlocks
John Bates
Reported 2011-06-29 19:25:24 PDT
Chromium bug: Compositing on a lost context causes latch deadlocks
Attachments
Patch (1.53 KB, patch)
2011-06-29 19:26 PDT, John Bates
no flags
Patch (1.81 KB, patch)
2011-06-30 15:51 PDT, John Bates
no flags
John Bates
Comment 1 2011-06-29 19:26:04 PDT
John Bates
Comment 2 2011-06-29 19:28:52 PDT
This bug is triggered by opening two tabs in the same renderer that support lost context recovery. Killing the GPU process would recreate the contexts. Then when the background tab is clicked, the first composite occurs on the old compositor context, thereby latching on a lost context and leading to deadlock.
James Robinson
Comment 3 2011-06-29 19:49:39 PDT
Comment on attachment 99211 [details] Patch isCompositorContextLost() is not a synchronous operation, so it seems like this will still be racy if the context is lost and the renderer side hasn't noticed yet - no?
John Bates
Comment 4 2011-06-30 09:44:16 PDT
(In reply to comment #3) > (From update of attachment 99211 [details]) > isCompositorContextLost() is not a synchronous operation, so it seems like this will still be racy if the context is lost and the renderer side hasn't noticed yet - no? This change fixes a condition that was actually not a race. The background tab has already lost its contexts (at GPU process kill time), and later on, it is displayed. Since the first thing we do is composite, we haven't discovered yet that the context was already lost, and we go ahead latching on it. For the cases where the compositor context is lost after this point, I think it's handled already. Here are the cases: - compositor context is lost due to error (memory, etc): GPU process detects error and kills whole GpuChannel, bringing down all child contexts as well. No deadlock because IPCs don't block on closed channels. - GPU crashed/killed: Also no deadlock because IPCs don't block on closed channels.
Kenneth Russell
Comment 5 2011-06-30 14:43:05 PDT
Comment on attachment 99211 [details] Patch Looks fine as long as it's been tested. I am also concerned about what would happen if the GPU process died immediately after the call to isCompositorContextLost, and also about what happens if we skip doing a render at this point, but it seems this is an improvement.
John Bates
Comment 6 2011-06-30 15:51:09 PDT
James Robinson
Comment 7 2011-06-30 16:06:01 PDT
Comment on attachment 99382 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=99382&action=review > Source/WebKit/chromium/ChangeLog:9 > + The cause of deadlock was that a setLatch command is added to the lost > + compositor GL context, and a waitLatch would never complete on a child context. > + This change checks whether the compositor context is in error state before > + doing the compositing. nit: in the future, put this after the bug link and a newline. pattern is: Reviewed by ... <newline> bug title bug URL <newline> long desc long desc long desc <newline> per-file stuff
WebKit Review Bot
Comment 8 2011-06-30 16:47:06 PDT
Comment on attachment 99382 [details] Patch Clearing flags on attachment: 99382 Committed r90185: <http://trac.webkit.org/changeset/90185>
WebKit Review Bot
Comment 9 2011-06-30 16:47:10 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.