Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2680313007:
cc: Remove resource freed tiles from tile manager pending gpu work set. (Closed)
Descriptioncc: Remove resource freed tiles from tile manager pending gpu work set.
CheckPendingGpuWorkTiles crashes because of nullptr dereference from
tiles whose resources have been freed.
R=ericrk@chromium.org,vmpstr@chromium.org
BUG=673434
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
Review-Url: https://codereview.chromium.org/2680313007
Cr-Commit-Position: refs/heads/master@{#449810}
Committed: https://chromium.googlesource.com/chromium/src/+/c5a9848cb8132fd80c83be857d4560a53ce11f21
Patch Set 1 #
Total comments: 2
Patch Set 2 : nits #Messages
Total messages: 19 (12 generated)
Description was changed from ========== cc: Remove resource freed tiles from tile manager pending gpu work set. CheckPendingGpuWorkTiles crashes because of use-after-free from tiles whose resources have been freed. R=ericrk@chromium.org,vmpstr@chromium.org BUG=673434 ========== to ========== cc: Remove resource freed tiles from tile manager pending gpu work set. CheckPendingGpuWorkTiles crashes because of use-after-free from tiles whose resources have been freed. R=ericrk@chromium.org,vmpstr@chromium.org BUG=673434 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
The CQ bit was checked by sunnyps@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
ptal
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
lgtm lgtm
Is this use-after-free or nullptr dereference? lgtm either way, thanks! https://codereview.chromium.org/2680313007/diff/1/cc/tiles/tile_manager.cc File cc/tiles/tile_manager.cc (right): https://codereview.chromium.org/2680313007/diff/1/cc/tiles/tile_manager.cc#ne... cc/tiles/tile_manager.cc:806: pending_gpu_work_tiles_.erase(tile); nit: might as well make it a part of the if
The CQ bit was checked by sunnyps@chromium.org to run a CQ dry run
Description was changed from ========== cc: Remove resource freed tiles from tile manager pending gpu work set. CheckPendingGpuWorkTiles crashes because of use-after-free from tiles whose resources have been freed. R=ericrk@chromium.org,vmpstr@chromium.org BUG=673434 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== cc: Remove resource freed tiles from tile manager pending gpu work set. CheckPendingGpuWorkTiles crashes because of nullptr dereference from tiles whose resources have been freed. R=ericrk@chromium.org,vmpstr@chromium.org BUG=673434 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Yes, this is a nullptr dereference. I've updated the CL description. Thanks! https://codereview.chromium.org/2680313007/diff/1/cc/tiles/tile_manager.cc File cc/tiles/tile_manager.cc (right): https://codereview.chromium.org/2680313007/diff/1/cc/tiles/tile_manager.cc#ne... cc/tiles/tile_manager.cc:806: pending_gpu_work_tiles_.erase(tile); On 2017/02/09 18:49:20, vmpstr wrote: > nit: might as well make it a part of the if Done. Also added a DCHECK in CheckPendingGpuWorkTiles.
The CQ bit was checked by sunnyps@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from vmpstr@chromium.org, ericrk@chromium.org Link to the patchset: https://codereview.chromium.org/2680313007/#ps20001 (title: "nits")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486770612048100,
"parent_rev": "f0e5e8156b96adf6099b0179c1b6be9d9d7721b4", "commit_rev":
"c5a9848cb8132fd80c83be857d4560a53ce11f21"}
Message was sent while issue was closed.
Description was changed from ========== cc: Remove resource freed tiles from tile manager pending gpu work set. CheckPendingGpuWorkTiles crashes because of nullptr dereference from tiles whose resources have been freed. R=ericrk@chromium.org,vmpstr@chromium.org BUG=673434 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== cc: Remove resource freed tiles from tile manager pending gpu work set. CheckPendingGpuWorkTiles crashes because of nullptr dereference from tiles whose resources have been freed. R=ericrk@chromium.org,vmpstr@chromium.org BUG=673434 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2680313007 Cr-Commit-Position: refs/heads/master@{#449810} Committed: https://chromium.googlesource.com/chromium/src/+/c5a9848cb8132fd80c83be857d45... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/c5a9848cb8132fd80c83be857d45... |
