New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 789742 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Fuchsia
Pri: 1
Type: Bug



Sign in to add a comment

GPU Mailbox leak in CanvasResource_Skia

Project Member Reported by vmi...@chromium.org, Nov 29 2017

Issue description

Chrome Version: r520210
OS: All except Mac.

In CanvasResource_Skia we create transferable resources from Skia SkImages, which associates GPU mailbox with the underlying GL texture.

When releasing CanvasResource_Skia, we remove the SkImage reference which allows Skia to recycle the image, but we never delete the associated mailbox.

As long as Skia keeps the GL texture alive, we leak GPU Mailboxes on the GPU service-side.

The mailbox should be disassociated by ProduceTextureDirectCHROMIUM() with texture id 0.

However this DCHECK in MailboxManagerSync (WebView only) should be removed.  The behavior looks OK but the DCHECK is invalid. 
 https://cs.chromium.org/chromium/src/gpu/command_buffer/service/mailbox_manager_sync.cc?q=MailboxManagerSync::ProduceTexture&sq=package:chromium&l=235
 

Comment 1 by vmi...@chromium.org, Nov 29 2017

Components: Blink>Canvas

Comment 2 by vmi...@chromium.org, Nov 30 2017

In case anyone started the same, I'm just pushing a fix for the MailboxManagerSync DCHECK here https://chromium-review.googlesource.com/#/c/chromium/src/+/798324.
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8982d52ad87c9ed9bb4edb1faaf0a7a46b70d5f6

commit 8982d52ad87c9ed9bb4edb1faaf0a7a46b70d5f6
Author: Victor Miura <vmiura@chromium.org>
Date: Thu Nov 30 03:06:15 2017

Remove an invalid DCHECK in MailboxManagerSync.

BUG= 789742 

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I47431033c267c2fc7824a9ff65326bd17d5709e8
Reviewed-on: https://chromium-review.googlesource.com/798324
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Victor Miura <vmiura@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520405}
[modify] https://crrev.com/8982d52ad87c9ed9bb4edb1faaf0a7a46b70d5f6/gpu/command_buffer/service/mailbox_manager_sync.cc

Comment 4 by junov@chromium.org, Nov 30 2017

Labels: M-64 ReleaseBlock-Stable

Comment 5 by junov@chromium.org, Nov 30 2017

Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/10de2b1dfda392d47caa4778fdb2df1640a34479

commit 10de2b1dfda392d47caa4778fdb2df1640a34479
Author: Justin Novosad <junov@chromium.org>
Date: Thu Nov 30 22:29:29 2017

Fix GPU Mailbox leak in CanvasResource_Skia

This change adds a call to ProduceTextureDirectCHROIMIUM to
disassociate a mailbox from its texture in case the texture
gets recycled by skia, in which case the mailbox name would be
leaked.

BUG= 789742 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ic07fc9897a2fa9e42bc5b1202e044c4171ca9f9a
Reviewed-on: https://chromium-review.googlesource.com/801496
Reviewed-by: Olivia Lai <xlai@chromium.org>
Commit-Queue: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520720}
[modify] https://crrev.com/10de2b1dfda392d47caa4778fdb2df1640a34479/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/10de2b1dfda392d47caa4778fdb2df1640a34479/third_party/WebKit/Source/platform/graphics/CanvasResource.cpp
[modify] https://crrev.com/10de2b1dfda392d47caa4778fdb2df1640a34479/third_party/WebKit/Source/platform/graphics/CanvasResource.h
[add] https://crrev.com/10de2b1dfda392d47caa4778fdb2df1640a34479/third_party/WebKit/Source/platform/graphics/CanvasResourceTest.cpp

Comment 7 by junov@chromium.org, Dec 1 2017

Status: (was: Started)
Fix landed.  Keeping this issue open until we know what the M-64 branch point is.

Comment 8 by junov@chromium.org, Dec 5 2017

Status: Fixed
M-64 branch was cut at 520840, so this fix made it.  No need to merge.  Closing issue.
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-64; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-64 label, otherwise remove Merge-TBD label. Thanks.
Labels: -Merge-TBD
Blockerbot: Thanks for trying to be helpful. Better luck next time.

Again: No merge required.

Sign in to add a comment