Fix graphics corruption caused by HWUI caches

Some caches(PatchCache, TextureCache, PathCache) for HWUI
uses deferred removal for their cache entries even though
actual resource objects are immediately freed by
ResourceCache.
For this reason, the uniqueness of a resource address in
the caches is not guaranteed in specific cases.
(Because malloc() can return the same address when malloc()
and free() called very frequently.)

So it can be possible the cache have two cache entries for
two different resources but the same memory address.
(Of course one of the resources is already freed.)
It also can be possible mGarbage vector in PatchCache has
duplicated addresses and this can lead to duplicated free
blocks in the free block list and graphics corruption.
(Deferred removal was implmeneted based on an assumption of
unique resource addresses.)

So this patch makes sure resource objects are freed after
the resources are removed from the caches to guarantee
the uniqueness of a resource address and prevent graphics
corruption.

Change-Id: I040f033a4fc783d2c4bc04b113589657c36fb15b
Signed-off-by: Sangkyu Lee <sk82.lee@lge.com>

4 files changed, 29 insertions, 15 deletions

diff --git a/libs/hwui/PatchCache.cpp b/libs/hwui/PatchCache.cpp
index dc0d98c..8a44604 100644
--- a/libs/hwui/PatchCache.cpp
+++ b/libs/hwui/PatchCache.cpp
@@ -129,7 +129,11 @@ void PatchCache::clearGarbage() {
         Mutex::Autolock _l(mLock);
         size_t count = mGarbage.size();
         for (size_t i = 0; i < count; i++) {
-            remove(patchesToRemove, mGarbage[i]);
+            Res_png_9patch* patch = mGarbage[i];
+            remove(patchesToRemove, patch);
+            // A Res_png_9patch is actually an array of byte that's larger
+            // than sizeof(Res_png_9patch). It must be freed as an array.
+            delete[] (int8_t*) patch;
         }
         mGarbage.clear();
     }
diff --git a/libs/hwui/PathCache.cpp b/libs/hwui/PathCache.cpp
index 5df6408..cf8adf8 100644
--- a/libs/hwui/PathCache.cpp
+++ b/libs/hwui/PathCache.cpp
@@ -395,7 +395,9 @@ void PathCache::clearGarbage() {
         Mutex::Autolock l(mLock);
         size_t count = mGarbage.size();
         for (size_t i = 0; i < count; i++) {
-            remove(pathsToRemove, mGarbage.itemAt(i));
+            const path_pair_t& pair = mGarbage.itemAt(i);
+            remove(pathsToRemove, pair);
+            delete pair.getFirst();
         }
         mGarbage.clear();
     }
diff --git a/libs/hwui/ResourceCache.cpp b/libs/hwui/ResourceCache.cpp
index 3f77021..d276a29 100644
--- a/libs/hwui/ResourceCache.cpp
+++ b/libs/hwui/ResourceCache.cpp
@@ -213,8 +213,9 @@ void ResourceCache::destructorLocked(SkPath* resource) {
         // If we're not tracking this resource, just delete it
         if (Caches::hasInstance()) {
             Caches::getInstance().pathCache.removeDeferred(resource);
+        } else {
+            delete resource;
         }
-        delete resource;
         return;
     }
     ref->destroyed = true;
@@ -235,8 +236,9 @@ void ResourceCache::destructorLocked(SkBitmap* resource) {
         // If we're not tracking this resource, just delete it
         if (Caches::hasInstance()) {
             Caches::getInstance().textureCache.removeDeferred(resource);
+        } else {
+            delete resource;
         }
-        delete resource;
         return;
     }
     ref->destroyed = true;
@@ -292,13 +294,14 @@ void ResourceCache::destructorLocked(Res_png_9patch* resource) {
     ssize_t index = mCache->indexOfKey(resource);
     ResourceReference* ref = index >= 0 ? mCache->valueAt(index) : NULL;
     if (ref == NULL) {
+        // If we're not tracking this resource, just delete it
         if (Caches::hasInstance()) {
             Caches::getInstance().patchCache.removeDeferred(resource);
+        } else {
+            // A Res_png_9patch is actually an array of byte that's larger
+            // than sizeof(Res_png_9patch). It must be freed as an array.
+            delete[] (int8_t*) resource;
         }
-        // If we're not tracking this resource, just delete it
-        // A Res_png_9patch is actually an array of byte that's larger
-        // than sizeof(Res_png_9patch). It must be freed as an array.
-        delete[] (int8_t*) resource;
         return;
     }
     ref->destroyed = true;
@@ -355,16 +358,18 @@ void ResourceCache::deleteResourceReferenceLocked(void* resource, ResourceRefere
                 SkBitmap* bitmap = (SkBitmap*) resource;
                 if (Caches::hasInstance()) {
                     Caches::getInstance().textureCache.removeDeferred(bitmap);
+                } else {
+                    delete bitmap;
                 }
-                delete bitmap;
             }
             break;
             case kPath: {
                 SkPath* path = (SkPath*) resource;
                 if (Caches::hasInstance()) {
                     Caches::getInstance().pathCache.removeDeferred(path);
+                } else {
+                    delete path;
                 }
-                delete path;
             }
             break;
             case kShader: {
@@ -380,11 +385,12 @@ void ResourceCache::deleteResourceReferenceLocked(void* resource, ResourceRefere
             case kNinePatch: {
                 if (Caches::hasInstance()) {
                     Caches::getInstance().patchCache.removeDeferred((Res_png_9patch*) resource);
+                } else {
+                    // A Res_png_9patch is actually an array of byte that's larger
+                    // than sizeof(Res_png_9patch). It must be freed as an array.
+                    int8_t* patch = (int8_t*) resource;
+                    delete[] patch;
                 }
-                // A Res_png_9patch is actually an array of byte that's larger
-                // than sizeof(Res_png_9patch). It must be freed as an array.
-                int8_t* patch = (int8_t*) resource;
-                delete[] patch;
             }
             break;
             case kLayer: {
diff --git a/libs/hwui/TextureCache.cpp b/libs/hwui/TextureCache.cpp
index ed0a79a..54a206b 100644
--- a/libs/hwui/TextureCache.cpp
+++ b/libs/hwui/TextureCache.cpp
@@ -184,7 +184,9 @@ void TextureCache::clearGarbage() {
     Mutex::Autolock _l(mLock);
     size_t count = mGarbage.size();
     for (size_t i = 0; i < count; i++) {
-        mCache.remove(mGarbage.itemAt(i));
+        const SkBitmap* bitmap = mGarbage.itemAt(i);
+        mCache.remove(bitmap);
+        delete bitmap;
     }
     mGarbage.clear();
 }