synchronous compositor: Deadlock in draw on scroll |
||||||||||
Issue descriptionFrom b/74560183. This call holds a lock, and can call back into the app: https://cs.chromium.org/chromium/src/content/browser/android/synchronous_compositor_sync_call_bridge.cc?rcl=e20ec0676c3adbdb4fb37acc17e692cb397f8ed0&l=187 If app then calls back to draw, then that deadlocks because same thread tries to acquire the same lock. Simple fix is to just not wrap the UpdateState in the lock.
,
Apr 10 2018
,
Apr 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8ca578ccfc770f1886100850452d9a94d47d06fa commit 8ca578ccfc770f1886100850452d9a94d47d06fa Author: Bo Liu <boliu@chromium.org> Date: Tue Apr 10 19:27:41 2018 sync compositor: Do not hold lock for UpdateState UpdateState can call back into app, which then call back into webview to draw again, which causes a deadlock. Also it's a bad pattern to have locks leak outside of a class anyway. Bug: 831248 Change-Id: I6e88fec0fdf38695074988b1cc52ebeecf45a206 Reviewed-on: https://chromium-review.googlesource.com/1005718 Reviewed-by: Dave Tapuska <dtapuska@chromium.org> Commit-Queue: Bo <boliu@chromium.org> Cr-Commit-Position: refs/heads/master@{#549611} [modify] https://crrev.com/8ca578ccfc770f1886100850452d9a94d47d06fa/content/browser/android/synchronous_compositor_sync_call_bridge.cc
,
Apr 10 2018
,
Apr 10 2018
This bug requires manual review: We are only 6 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 10 2018
Merge approved upon verification of the fix in canary
,
Apr 10 2018
boliu@, I see that you had mentioned in the b/74560183 that u repro'ed ANR by Simulating the stack in a simple app. We were not able to repro it in our testing. So, are there any steps, where we can repro & verify on fixed build?
,
Apr 10 2018
Trying to scroll using this shell apk will immediately lock up without the fix. Note that apk is only for reproducing this ANR and nothing else. Even with the fix, it's super slow, and still might crash.
,
Apr 11 2018
Fix issue verified on latest Pixel 2 XL, from the steps #3 in b/74560183. Thanks!
,
Apr 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/06de90b777451321f6ca696edf3ec59d2137830e commit 06de90b777451321f6ca696edf3ec59d2137830e Author: Bo Liu <boliu@chromium.org> Date: Wed Apr 11 17:24:15 2018 [Merge M66] sync compositor: Do not hold lock for UpdateState UpdateState can call back into app, which then call back into webview to draw again, which causes a deadlock. Also it's a bad pattern to have locks leak outside of a class anyway. Bug: 831248 Change-Id: I6e88fec0fdf38695074988b1cc52ebeecf45a206 Reviewed-on: https://chromium-review.googlesource.com/1005718 Reviewed-by: Dave Tapuska <dtapuska@chromium.org> Commit-Queue: Bo <boliu@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#549611}(cherry picked from commit 8ca578ccfc770f1886100850452d9a94d47d06fa) Reviewed-on: https://chromium-review.googlesource.com/1007743 Reviewed-by: Bo <boliu@chromium.org> Cr-Commit-Position: refs/branch-heads/3359@{#682} Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276} [modify] https://crrev.com/06de90b777451321f6ca696edf3ec59d2137830e/content/browser/android/synchronous_compositor_sync_call_bridge.cc
,
Apr 11 2018
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by boliu@chromium.org
, Apr 10 2018