New issue
Advanced search Search tips

Issue 878483 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 8
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

GLRenderer destructor frees and then reallocates command buffer ring buffer memory.

Project Member Reported by kylec...@chromium.org, Aug 28

Issue description

The destruction order in the GLRenderer destructor seems less than ideal. It can free and then reallocate the command buffer ring buffer memory. This isn't necessarily incorrect it's just not ideal.

~GLRenderer() will call ContextCacheController::ClientBecameNotVisible() during destruction at [1]. That will call GLES2Implementation::SetAggressivelyFreeResources() and then ImplementationBase::FreeEverything() which deletes the ring buffer. 

Later on in ~GLRenderer() the SyncQueryCollection member variable is destroyed. ~SyncQueryCollection() destroys the SyncQuery objects it owns. ~SyncQuery() can make a GL call at [2] which reallocates the ring buffer than was just destroyed to hold the GL command.

[1] https://cs.chromium.org/chromium/src/components/viz/service/display/gl_renderer.cc?l=348&rcl=352ee5a7003e0699c82bf86d68be606fb82eb179
[2] https://cs.chromium.org/chromium/src/components/viz/service/display/sync_query_collection.cc?l=26&rcl=352ee5a7003e0699c82bf86d68be606fb82eb179
 
Owner: sgilhuly@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 7

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

commit c43feb50891f843c5ee349180fb3e37fe445d3d9
Author: Sean Gilhuly <sgilhuly@chromium.org>
Date: Mon Jan 07 19:22:36 2019

Hold on to resources during GLRenderer cleanup

Resources are being freed too aggressively in ~GLRenderer. This was
causing command buffer memory to to be freed while some objects still
had GL calls to make, as late as glDeleteTextures in
DisplayResourceProvider::DeleteResourceInternal().

Persist the visibility token that GLRenderer releases until the
ContextCacheController is destroyed, where the token can be released and
command buffer resources freed.

Bug:  878483 
Change-Id: I558bfa9f14e7b50d51180802c4186695b847fe87
Reviewed-on: https://chromium-review.googlesource.com/c/1383320
Reviewed-by: Eric Karl <ericrk@chromium.org>
Commit-Queue: Sean Gilhuly <sgilhuly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620420}
[modify] https://crrev.com/c43feb50891f843c5ee349180fb3e37fe445d3d9/components/viz/common/gpu/context_cache_controller.cc
[modify] https://crrev.com/c43feb50891f843c5ee349180fb3e37fe445d3d9/components/viz/common/gpu/context_cache_controller.h
[modify] https://crrev.com/c43feb50891f843c5ee349180fb3e37fe445d3d9/components/viz/common/gpu/context_cache_controller_unittest.cc
[modify] https://crrev.com/c43feb50891f843c5ee349180fb3e37fe445d3d9/components/viz/service/display/gl_renderer.cc

Status: Fixed (was: Assigned)

Sign in to add a comment