Free resources only from the GL context thread.

Bug #3179882

Resources were freed following garbage collections on a worker thread.
This worker thread had no EGL context, which would cause the renderer
to incorrectly assume that the memory was liberated.

Change-Id: Ifdb51f94ddf42641e8654522787bfac532976c7c

12 files changed, 121 insertions, 65 deletions

diff --git a/libs/hwui/Caches.cpp b/libs/hwui/Caches.cpp
index 2b498c6..40dd117 100644
--- a/libs/hwui/Caches.cpp
+++ b/libs/hwui/Caches.cpp
@@ -95,6 +95,16 @@ void Caches::dumpMemoryUsage() {
 }
 
 ///////////////////////////////////////////////////////////////////////////////
+// Memory management
+///////////////////////////////////////////////////////////////////////////////
+
+void Caches::clearGarbage() {
+    textureCache.clearGarbage();
+    gradientCache.clearGarbage();
+    pathCache.clearGarbage();
+}
+
+///////////////////////////////////////////////////////////////////////////////
 // VBO
 ///////////////////////////////////////////////////////////////////////////////
 
diff --git a/libs/hwui/Caches.h b/libs/hwui/Caches.h
index 2779dfd..e8ced9c 100644
--- a/libs/hwui/Caches.h
+++ b/libs/hwui/Caches.h
@@ -101,6 +101,12 @@ public:
     }
 
     /**
+     * Call this on each frame to ensure that garbage is deleted from
+     * GPU memory.
+     */
+    void clearGarbage();
+
+    /**
      * Binds the VBO used to render simple textured quads.
      */
     void bindMeshBuffer();
diff --git a/libs/hwui/DisplayListRenderer.cpp b/libs/hwui/DisplayListRenderer.cpp
index d08df44..77d628a 100644
--- a/libs/hwui/DisplayListRenderer.cpp
+++ b/libs/hwui/DisplayListRenderer.cpp
@@ -153,13 +153,13 @@ DisplayList::~DisplayList() {
     mPaints.clear();
 
     for (size_t i = 0; i < mMatrices.size(); i++) {
-        delete  mMatrices.itemAt(i);
+        delete mMatrices.itemAt(i);
     }
     mMatrices.clear();
 
     if (mPathHeap) {
         for (int i = 0; i < mPathHeap->count(); i++) {
-            caches.pathCache.remove(&(*mPathHeap)[i]);
+            caches.pathCache.removeDeferred(&(*mPathHeap)[i]);
         }
         mPathHeap->safeUnref();
     }
diff --git a/libs/hwui/GradientCache.cpp b/libs/hwui/GradientCache.cpp
index 97f4cb4..a37964e 100644
--- a/libs/hwui/GradientCache.cpp
+++ b/libs/hwui/GradientCache.cpp
@@ -54,7 +54,6 @@ GradientCache::GradientCache(uint32_t maxByteSize):
 }
 
 GradientCache::~GradientCache() {
-    Mutex::Autolock _l(mLock);
     mCache.clear();
 }
 
@@ -63,17 +62,14 @@ GradientCache::~GradientCache() {
 ///////////////////////////////////////////////////////////////////////////////
 
 uint32_t GradientCache::getSize() {
-    Mutex::Autolock _l(mLock);
     return mSize;
 }
 
 uint32_t GradientCache::getMaxSize() {
-    Mutex::Autolock _l(mLock);
     return mMaxSize;
 }
 
 void GradientCache::setMaxSize(uint32_t maxSize) {
-    Mutex::Autolock _l(mLock);
     mMaxSize = maxSize;
     while (mSize > mMaxSize) {
         mCache.removeOldest();
@@ -102,17 +98,28 @@ void GradientCache::operator()(SkShader*& shader, Texture*& texture) {
 ///////////////////////////////////////////////////////////////////////////////
 
 Texture* GradientCache::get(SkShader* shader) {
-    Mutex::Autolock _l(mLock);
     return mCache.get(shader);
 }
 
 void GradientCache::remove(SkShader* shader) {
-    Mutex::Autolock _l(mLock);
     mCache.remove(shader);
 }
 
-void GradientCache::clear() {
+void GradientCache::removeDeferred(SkShader* shader) {
     Mutex::Autolock _l(mLock);
+    mGarbage.push(shader);
+}
+
+void GradientCache::clearGarbage() {
+    Mutex::Autolock _l(mLock);
+    size_t count = mGarbage.size();
+    for (size_t i = 0; i < count; i++) {
+        mCache.remove(mGarbage.itemAt(i));
+    }
+    mGarbage.clear();
+}
+
+void GradientCache::clear() {
     mCache.clear();
 }
 
@@ -138,21 +145,17 @@ Texture* GradientCache::addLinearGradient(SkShader* shader, uint32_t* colors,
 
     canvas.drawRectCoords(0.0f, 0.0f, bitmap.width(), 1.0f, p);
 
-    mLock.lock();
     // Asume the cache is always big enough
     const uint32_t size = bitmap.rowBytes() * bitmap.height();
     while (mSize + size > mMaxSize) {
         mCache.removeOldest();
     }
-    mLock.unlock();
 
     Texture* texture = new Texture;
     generateTexture(&bitmap, texture);
 
-    mLock.lock();
     mSize += size;
     mCache.put(shader, texture);
-    mLock.unlock();
 
     return texture;
 }
diff --git a/libs/hwui/GradientCache.h b/libs/hwui/GradientCache.h
index c9553f4..30da462 100644
--- a/libs/hwui/GradientCache.h
+++ b/libs/hwui/GradientCache.h
@@ -19,6 +19,8 @@
 
 #include <SkShader.h>
 
+#include <utils/Vector.h>
+
 #include "Texture.h"
 #include "utils/GenerationCache.h"
 
@@ -53,11 +55,20 @@ public:
      */
     Texture* get(SkShader* shader);
     /**
-     * Removes the texture associated with the specified shader. Returns NULL
-     * if the texture cannot be found. Upon remove the texture is freed.
+     * Removes the texture associated with the specified shader.
+     * Upon remove the texture is freed.
      */
     void remove(SkShader* shader);
     /**
+     * Removes the texture associated with the specified shader. This is meant
+     * to be called from threads that are not the EGL context thread.
+     */
+    void removeDeferred(SkShader* shader);
+    /**
+     * Process deferred removals.
+     */
+    void clearGarbage();
+    /**
      * Clears the cache. This causes all textures to be deleted.
      */
     void clear();
@@ -83,10 +94,7 @@ private:
     uint32_t mSize;
     uint32_t mMaxSize;
 
-    /**
-     * Used to access mCache and mSize. All methods are accessed from a single
-     * thread except for remove().
-     */
+    Vector<SkShader*> mGarbage;
     mutable Mutex mLock;
 }; // class GradientCache
 
diff --git a/libs/hwui/OpenGLRenderer.cpp b/libs/hwui/OpenGLRenderer.cpp
index acce1a2..ad72584 100644
--- a/libs/hwui/OpenGLRenderer.cpp
+++ b/libs/hwui/OpenGLRenderer.cpp
@@ -135,6 +135,8 @@ void OpenGLRenderer::setViewport(int width, int height) {
 }
 
 void OpenGLRenderer::prepare(bool opaque) {
+    mCaches.clearGarbage();
+
     mSnapshot = new Snapshot(mFirstSnapshot,
             SkCanvas::kMatrix_SaveFlag | SkCanvas::kClip_SaveFlag);
     mSaveCount = 1;
diff --git a/libs/hwui/PathCache.cpp b/libs/hwui/PathCache.cpp
index 7ff26dc..8740a64 100644
--- a/libs/hwui/PathCache.cpp
+++ b/libs/hwui/PathCache.cpp
@@ -53,7 +53,6 @@ PathCache::PathCache(uint32_t maxByteSize):
 }
 
 PathCache::~PathCache() {
-    Mutex::Autolock _l(mLock);
     mCache.clear();
 }
 
@@ -72,17 +71,14 @@ void PathCache::init() {
 ///////////////////////////////////////////////////////////////////////////////
 
 uint32_t PathCache::getSize() {
-    Mutex::Autolock _l(mLock);
     return mSize;
 }
 
 uint32_t PathCache::getMaxSize() {
-    Mutex::Autolock _l(mLock);
     return mMaxSize;
 }
 
 void PathCache::setMaxSize(uint32_t maxSize) {
-    Mutex::Autolock _l(mLock);
     mMaxSize = maxSize;
     while (mSize > mMaxSize) {
         mCache.removeOldest();
@@ -94,6 +90,14 @@ void PathCache::setMaxSize(uint32_t maxSize) {
 ///////////////////////////////////////////////////////////////////////////////
 
 void PathCache::operator()(PathCacheEntry& path, PathTexture*& texture) {
+    removeTexture(texture);
+}
+
+///////////////////////////////////////////////////////////////////////////////
+// Caching
+///////////////////////////////////////////////////////////////////////////////
+
+void PathCache::removeTexture(PathTexture* texture) {
     if (texture) {
         const uint32_t size = texture->width * texture->height;
         mSize -= size;
@@ -109,39 +113,46 @@ void PathCache::operator()(PathCacheEntry& path, PathTexture*& texture) {
     }
 }
 
-///////////////////////////////////////////////////////////////////////////////
-// Caching
-///////////////////////////////////////////////////////////////////////////////
-
 void PathCache::remove(SkPath* path) {
-    Mutex::Autolock _l(mLock);
-
     // TODO: Linear search...
     Vector<uint32_t> pathsToRemove;
     for (uint32_t i = 0; i < mCache.size(); i++) {
         if (mCache.getKeyAt(i).path == path) {
             pathsToRemove.push(i);
+            removeTexture(mCache.getValueAt(i));
         }
     }
 
+    mCache.setOnEntryRemovedListener(NULL);
     for (size_t i = 0; i < pathsToRemove.size(); i++) {
         mCache.removeAt(pathsToRemove.itemAt(i));
     }
+    mCache.setOnEntryRemovedListener(this);
+}
+
+void PathCache::removeDeferred(SkPath* path) {
+    Mutex::Autolock _l(mLock);
+    mGarbage.push(path);
+}
+
+void PathCache::clearGarbage() {
+    Mutex::Autolock _l(mLock);
+    size_t count = mGarbage.size();
+    for (size_t i = 0; i < count; i++) {
+        remove(mGarbage.itemAt(i));
+    }
+    mGarbage.clear();
 }
 
 PathTexture* PathCache::get(SkPath* path, SkPaint* paint) {
     PathCacheEntry entry(path, paint);
 
-    mLock.lock();
     PathTexture* texture = mCache.get(entry);
-    mLock.unlock();
 
     if (!texture) {
         texture = addTexture(entry, path, paint);
     } else if (path->getGenerationID() != texture->generation) {
-        mLock.lock();
         mCache.remove(entry);
-        mLock.unlock();
         texture = addTexture(entry, path, paint);
     }
 
@@ -167,11 +178,9 @@ PathTexture* PathCache::addTexture(const PathCacheEntry& entry,
     const uint32_t size = width * height;
     // Don't even try to cache a bitmap that's bigger than the cache
     if (size < mMaxSize) {
-        mLock.lock();
         while (mSize + size > mMaxSize) {
             mCache.removeOldest();
         }
-        mLock.unlock();
     }
 
     PathTexture* texture = new PathTexture;
@@ -200,7 +209,6 @@ PathTexture* PathCache::addTexture(const PathCacheEntry& entry,
     generateTexture(bitmap, texture);
 
     if (size < mMaxSize) {
-        mLock.lock();
         mSize += size;
         PATH_LOGD("PathCache::get: create path: name, size, mSize = %d, %d, %d",
                 texture->id, size, mSize);
@@ -208,7 +216,6 @@ PathTexture* PathCache::addTexture(const PathCacheEntry& entry,
             LOGD("Path created, size = %d", size);
         }
         mCache.put(entry, texture);
-        mLock.unlock();
     } else {
         texture->cleanup = true;
     }
@@ -217,7 +224,6 @@ PathTexture* PathCache::addTexture(const PathCacheEntry& entry,
 }
 
 void PathCache::clear() {
-    Mutex::Autolock _l(mLock);
     mCache.clear();
 }
 
diff --git a/libs/hwui/PathCache.h b/libs/hwui/PathCache.h
index 0193f43..ae2e55d 100644
--- a/libs/hwui/PathCache.h
+++ b/libs/hwui/PathCache.h
@@ -21,6 +21,8 @@
 #include <SkPaint.h>
 #include <SkPath.h>
 
+#include <utils/Vector.h>
+
 #include "Debug.h"
 #include "Texture.h"
 #include "utils/Compare.h"
@@ -146,6 +148,15 @@ public:
      * Removes an entry.
      */
     void remove(SkPath* path);
+    /**
+     * Removes the specified path. This is meant to be called from threads
+     * that are not the EGL context thread.
+     */
+    void removeDeferred(SkPath* path);
+    /**
+     * Process deferred removals.
+     */
+    void clearGarbage();
 
     /**
      * Sets the maximum size of the cache in bytes.
@@ -166,6 +177,8 @@ private:
      */
     void generateTexture(SkBitmap& bitmap, Texture* texture);
 
+    void removeTexture(PathTexture* texture);
+
     PathTexture* addTexture(const PathCacheEntry& entry, const SkPath *path, const SkPaint* paint);
 
     void init();
@@ -178,10 +191,7 @@ private:
 
     bool mDebugEnabled;
 
-    /**
-     * Used to access mCache and mSize. All methods are accessed from a single
-     * thread except for remove().
-     */
+    Vector<SkPath*> mGarbage;
     mutable Mutex mLock;
 }; // class PathCache
 
diff --git a/libs/hwui/ResourceCache.cpp b/libs/hwui/ResourceCache.cpp
index 47c5d48..9f18948 100644
--- a/libs/hwui/ResourceCache.cpp
+++ b/libs/hwui/ResourceCache.cpp
@@ -126,7 +126,7 @@ void ResourceCache::destructor(SkBitmap* resource) {
     if (ref == NULL) {
         // If we're not tracking this resource, just delete it
         if (Caches::hasInstance()) {
-            Caches::getInstance().textureCache.remove(resource);
+            Caches::getInstance().textureCache.removeDeferred(resource);
         }
         delete resource;
         return;
@@ -143,7 +143,7 @@ void ResourceCache::destructor(SkiaShader* resource) {
     if (ref == NULL) {
         // If we're not tracking this resource, just delete it
         if (Caches::hasInstance()) {
-            Caches::getInstance().gradientCache.remove(resource->getSkShader());
+            Caches::getInstance().gradientCache.removeDeferred(resource->getSkShader());
         }
         delete resource;
         return;
@@ -179,7 +179,7 @@ void ResourceCache::deleteResourceReference(void* resource, ResourceReference* r
             {
                 SkBitmap* bitmap = (SkBitmap*)resource;
                 if (Caches::hasInstance()) {
-                    Caches::getInstance().textureCache.remove(bitmap);
+                    Caches::getInstance().textureCache.removeDeferred(bitmap);
                 }
                 delete bitmap;
             }
@@ -188,7 +188,7 @@ void ResourceCache::deleteResourceReference(void* resource, ResourceReference* r
             {
                 SkiaShader* shader = (SkiaShader*)resource;
                 if (Caches::hasInstance()) {
-                    Caches::getInstance().gradientCache.remove(shader->getSkShader());
+                    Caches::getInstance().gradientCache.removeDeferred(shader->getSkShader());
                 }
                 delete shader;
             }
diff --git a/libs/hwui/TextureCache.cpp b/libs/hwui/TextureCache.cpp
index 1be6868..ebecebe 100644
--- a/libs/hwui/TextureCache.cpp
+++ b/libs/hwui/TextureCache.cpp
@@ -53,7 +53,6 @@ TextureCache::TextureCache(uint32_t maxByteSize):
 }
 
 TextureCache::~TextureCache() {
-    Mutex::Autolock _l(mLock);
     mCache.clear();
 }
 
@@ -71,17 +70,14 @@ void TextureCache::init() {
 ///////////////////////////////////////////////////////////////////////////////
 
 uint32_t TextureCache::getSize() {
-    Mutex::Autolock _l(mLock);
     return mSize;
 }
 
 uint32_t TextureCache::getMaxSize() {
-    Mutex::Autolock _l(mLock);
     return mMaxSize;
 }
 
 void TextureCache::setMaxSize(uint32_t maxSize) {
-    Mutex::Autolock _l(mLock);
     mMaxSize = maxSize;
     while (mSize > mMaxSize) {
         mCache.removeOldest();
@@ -111,9 +107,7 @@ void TextureCache::operator()(SkBitmap*& bitmap, Texture*& texture) {
 ///////////////////////////////////////////////////////////////////////////////
 
 Texture* TextureCache::get(SkBitmap* bitmap) {
-    mLock.lock();
     Texture* texture = mCache.get(bitmap);
-    mLock.unlock();
 
     if (!texture) {
         if (bitmap->width() > mMaxTextureSize || bitmap->height() > mMaxTextureSize) {
@@ -124,11 +118,9 @@ Texture* TextureCache::get(SkBitmap* bitmap) {
         const uint32_t size = bitmap->rowBytes() * bitmap->height();
         // Don't even try to cache a bitmap that's bigger than the cache
         if (size < mMaxSize) {
-            mLock.lock();
             while (mSize + size > mMaxSize) {
                 mCache.removeOldest();
             }
-            mLock.unlock();
         }
 
         texture = new Texture;
@@ -136,7 +128,6 @@ Texture* TextureCache::get(SkBitmap* bitmap) {
         generateTexture(bitmap, texture, false);
 
         if (size < mMaxSize) {
-            mLock.lock();
             mSize += size;
             TEXTURE_LOGD("TextureCache::get: create texture(%p): name, size, mSize = %d, %d, %d",
                      bitmap, texture->id, size, mSize);
@@ -144,7 +135,6 @@ Texture* TextureCache::get(SkBitmap* bitmap) {
                 LOGD("Texture created, size = %d", size);
             }
             mCache.put(bitmap, texture);
-            mLock.unlock();
         } else {
             texture->cleanup = true;
         }
@@ -156,12 +146,24 @@ Texture* TextureCache::get(SkBitmap* bitmap) {
 }
 
 void TextureCache::remove(SkBitmap* bitmap) {
-    Mutex::Autolock _l(mLock);
     mCache.remove(bitmap);
 }
 
-void TextureCache::clear() {
+void TextureCache::removeDeferred(SkBitmap* bitmap) {
     Mutex::Autolock _l(mLock);
+    mGarbage.push(bitmap);
+}
+
+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));
+    }
+    mGarbage.clear();
+}
+
+void TextureCache::clear() {
     mCache.clear();
     TEXTURE_LOGD("TextureCache:clear(), miSize = %d", mSize);
 }
diff --git a/libs/hwui/TextureCache.h b/libs/hwui/TextureCache.h
index 27693df..f7707f7 100644
--- a/libs/hwui/TextureCache.h
+++ b/libs/hwui/TextureCache.h
@@ -19,6 +19,8 @@
 
 #include <SkBitmap.h>
 
+#include <utils/Vector.h>
+
 #include "Debug.h"
 #include "Texture.h"
 #include "utils/GenerationCache.h"
@@ -64,11 +66,21 @@ public:
      */
     Texture* get(SkBitmap* bitmap);
     /**
-     * Removes the texture associated with the specified bitmap. Returns NULL
-     * if the texture cannot be found. Upon remove the texture is freed.
+     * Removes the texture associated with the specified bitmap.
+     * Upon remove the texture is freed.
      */
     void remove(SkBitmap* bitmap);
     /**
+     * Removes the texture associated with the specified bitmap. This is meant
+     * to be called from threads that are not the EGL context thread.
+     */
+    void removeDeferred(SkBitmap* bitmap);
+    /**
+     * Process deferred removals.
+     */
+    void clearGarbage();
+
+    /**
      * Clears the cache. This causes all textures to be deleted.
      */
     void clear();
@@ -109,10 +121,7 @@ private:
 
     bool mDebugEnabled;
 
-    /**
-     * Used to access mCache and mSize. All methods are accessed from a single
-     * thread except for remove().
-     */
+    Vector<SkBitmap*> mGarbage;
     mutable Mutex mLock;
 }; // class TextureCache
 
diff --git a/libs/hwui/utils/GenerationCache.h b/libs/hwui/utils/GenerationCache.h
index 2e76236..42e6d9b 100644
--- a/libs/hwui/utils/GenerationCache.h
+++ b/libs/hwui/utils/GenerationCache.h
@@ -130,7 +130,7 @@ K GenerationCache<K, V>::getKeyAt(uint32_t index) const {
 
 template<typename K, typename V>
 V GenerationCache<K, V>::getValueAt(uint32_t index) const {
-    return mCache.valueAt(index);
+    return mCache.valueAt(index)->value;
 }
 
 template<typename K, typename V>