Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 36fad8f6 authored by Sangkyu Lee's avatar Sangkyu Lee Committed by Chris Craik
Browse files

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: default avatarSangkyu Lee <sk82.lee@lge.com>
parent 2ba70fd4

libs/hwui/PatchCache.cpp

+5 −1
Original line number Diff line number Diff line
@@ -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();

    }

libs/hwui/PathCache.cpp

+3 −1
Original line number Diff line number Diff line
@@ -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();

    }

libs/hwui/ResourceCache.cpp

+18 −12
Original line number Diff line number Diff line
@@ -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;

        }

        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;

        }

        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);

        }

        // If we're not tracking this resource, just delete it

        } 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;

        }

        return;

    }

    ref->destroyed = true;
@@ -355,17 +358,19 @@ void ResourceCache::deleteResourceReferenceLocked(void* resource, ResourceRefere 
                SkBitmap* bitmap = (SkBitmap*) resource;

                if (Caches::hasInstance()) {

                    Caches::getInstance().textureCache.removeDeferred(bitmap);

                }

                } else {

                    delete bitmap;

                }

            }

            break;

            case kPath: {

                SkPath* path = (SkPath*) resource;

                if (Caches::hasInstance()) {

                    Caches::getInstance().pathCache.removeDeferred(path);

                }

                } else {

                    delete path;

                }

            }

            break;

            case kShader: {

                SkiaShader* shader = (SkiaShader*) resource;
@@ -380,12 +385,13 @@ 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;

                }

            }

            break;

            case kLayer: {

                Layer* layer = (Layer*) resource;

libs/hwui/TextureCache.cpp

+3 −1
Original line number Diff line number Diff line
@@ -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();

}