From 6056e1027107aaa15f51a5ed775ff14c6b664ca3 Mon Sep 17 00:00:00 2001 From: Jens Gulin Date: Tue, 4 Feb 2014 17:38:02 +0100 Subject: [PATCH] Solve three memory leaks related to PatchCache A Patch can be fairly large, holding bitmap data, but is also frequently leaked which adds to the severity. The feature is used in many important processes such as Home, SystemUI and Chrome. The following leaks are solved: 1. The Patch itself was not always freed. PatchCache::removeDeferred() can mark patches to be cared for by PatchCache::clearGarbage(). But mCache.remove() would only destroy the container and the pointer, not the Patch object itself. 2. The vertices stored in the Patch at Patch::createMesh() would always leak. The empty/default destructor in Patch would not properly destroy "vertices" since it's just a pointer. 3. A BufferBlock that's added to the mFreeBlocks in PatchCache could leak. The leak happened when a patch later needed the entire free block, because the object was removed from the list but never deleted in PatchCache::setupMesh(). Change-Id: I41e60824479230b67426fc546d3dbff294c8891f --- libs/hwui/Patch.cpp | 1 + libs/hwui/PatchCache.cpp | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/libs/hwui/Patch.cpp b/libs/hwui/Patch.cpp index b2148b04ba92..442e9ba616cf 100644 --- a/libs/hwui/Patch.cpp +++ b/libs/hwui/Patch.cpp @@ -36,6 +36,7 @@ Patch::Patch(): vertices(NULL), verticesCount(0), indexCount(0), hasEmptyQuads(f } Patch::~Patch() { + delete[] vertices; } /////////////////////////////////////////////////////////////////////////////// diff --git a/libs/hwui/PatchCache.cpp b/libs/hwui/PatchCache.cpp index 8a446043b70f..2f2debc89dd1 100644 --- a/libs/hwui/PatchCache.cpp +++ b/libs/hwui/PatchCache.cpp @@ -119,6 +119,17 @@ void PatchCache::remove(Vector& patchesToRemove, Res_png_9patch* p void PatchCache::removeDeferred(Res_png_9patch* patch) { Mutex::Autolock _l(mLock); + + // Assert that patch is not already garbage + size_t count = mGarbage.size(); + for (size_t i = 0; i < count; i++) { + if (patch == mGarbage[i]) { + patch = NULL; + break; + } + } + LOG_ALWAYS_FATAL_IF(patch == NULL); + mGarbage.push(patch); } @@ -143,8 +154,8 @@ void PatchCache::clearGarbage() { for (size_t i = 0; i < patchesToRemove.size(); i++) { const patch_pair_t& pair = patchesToRemove[i]; - // Add a new free block to the list - const Patch* patch = pair.getSecond(); + // Release the patch and mark the space in the free list + Patch* patch = pair.getSecond(); BufferBlock* block = new BufferBlock(patch->offset, patch->getSize()); block->next = mFreeBlocks; mFreeBlocks = block; @@ -152,6 +163,7 @@ void PatchCache::clearGarbage() { mSize -= patch->getSize(); mCache.remove(*pair.getFirst()); + delete patch; } #if DEBUG_PATCHES @@ -216,6 +228,7 @@ void PatchCache::setupMesh(Patch* newMesh, TextureVertex* vertices) { } else { mFreeBlocks = block->next; } + delete block; } else { // Resize the block now that it's occupied block->offset += size; -- 2.11.0