Fix memory leak of resources caused by incorrect external references. (…

…#325)

src/gpu/DrawingManager.h

@@ -52,7 +52,7 @@ class DrawingManager {
 
  private:
   Context* context = nullptr;
-  UniqueKeyMap<ResourceTask*> resourceTaskMap = {};
+  ResourceKeyMap<ResourceTask*> resourceTaskMap = {};
   std::unordered_set<std::shared_ptr<RenderTargetProxy>> needResolveTargets = {};
   std::vector<std::shared_ptr<ResourceTask>> resourceTasks = {};
   std::vector<std::shared_ptr<RenderTask>> renderTasks = {};

src/gpu/ProxyProvider.cpp

@@ -208,7 +208,7 @@ void ProxyProvider::changeProxyKey(std::shared_ptr<ResourceProxy> proxy, const U
 }
 
 void ProxyProvider::purgeExpiredProxies() {
-  std::vector<const UniqueKey*> keys = {};
+  std::vector<const ResourceKey*> keys = {};
   for (auto& pair : proxyMap) {
     if (pair.second.expired()) {
       keys.push_back(&pair.first);

src/gpu/ProxyProvider.h

@@ -131,7 +131,7 @@ class ProxyProvider {
 
  private:
   Context* context = nullptr;
-  UniqueKeyMap<std::weak_ptr<ResourceProxy>> proxyMap = {};
+  ResourceKeyMap<std::weak_ptr<ResourceProxy>> proxyMap = {};
 
   static UniqueKey GetProxyKey(const UniqueKey& uniqueKey, uint32_t renderFlags);
 

src/gpu/ResourceCache.h

@@ -112,8 +112,8 @@ class ResourceCache {
   size_t purgeableBytes = 0;
   std::list<Resource*> nonpurgeableResources = {};
   std::list<Resource*> purgeableResources = {};
-  ScratchKeyMap<std::vector<Resource*>> scratchKeyMap = {};
-  UniqueKeyMap<Resource*> uniqueKeyMap = {};
+  ResourceKeyMap<std::vector<Resource*>> scratchKeyMap = {};
+  ResourceKeyMap<Resource*> uniqueKeyMap = {};
 
   static void AddToList(std::list<Resource*>& list, Resource* resource);
   static void RemoveFromList(std::list<Resource*>& list, Resource* resource);

src/gpu/ResourceKey.cpp

@@ -52,25 +52,28 @@ ResourceKey::ResourceKey(ResourceKey&& key) noexcept : data(key.data), count(key
   key.count = 0;
 }
 
-bool ResourceKey::equal(const ResourceKey& that) const {
-  if (count != that.count) {
-    return false;
+ResourceKey& ResourceKey::operator=(const ResourceKey& that) {
+  if (this == &that) {
+    return *this;
   }
-  return memcmp(data, that.data, count * sizeof(uint32_t)) == 0;
+  delete[] data;
+  data = CopyData(that.data, that.count);
+  count = that.count;
+  return *this;
 }
 
-void ResourceKey::copy(const ResourceKey& that) {
-  if (data == that.data) {
-    return;
+bool ResourceKey::operator==(const ResourceKey& that) const {
+  if (count != that.count) {
+    return false;
   }
-  data = CopyData(that.data, that.count);
-  count = that.count;
+  return memcmp(data, that.data, count * sizeof(uint32_t)) == 0;
 }
 
 ScratchKey::ScratchKey(uint32_t* data, size_t count) : ResourceKey(data, count) {
 }
 
 ScratchKey& ScratchKey::operator=(const BytesKey& that) {
+  delete[] data;
   data = CopyData(that.data(), that.size(), 1);
   if (data != nullptr) {
     data[0] = HashRange(that.data(), that.size());
@@ -98,7 +101,7 @@ UniqueKey UniqueKey::Combine(const UniqueKey& uniqueKey, const BytesKey& bytesKe
     return uniqueKey;
   }
   if (uniqueKey.count > 2) {
-    memcpy(data + 2, uniqueKey.data + 2, uniqueKey.count - 2);
+    memcpy(data + 2, uniqueKey.data + 2, (uniqueKey.count - 2) * sizeof(uint32_t));
   }
   auto count = bytesKey.size() + offset;
   auto domain = uniqueKey.uniqueDomain;
@@ -175,7 +178,7 @@ UniqueKey& UniqueKey::operator=(const UniqueKey& key) {
   if (uniqueDomain != nullptr) {
     uniqueDomain->addReference();
   }
-  copy(key);
+  ResourceKey::operator=(key);
   return *this;
 }
 

src/gpu/ResourceKey.h

@@ -31,6 +31,10 @@ namespace tgfx {
  */
 class ResourceKey {
  public:
+  ResourceKey() = default;
+
+  ResourceKey(const ResourceKey& that);
+
   virtual ~ResourceKey();
 
   /**
@@ -47,23 +51,32 @@ class ResourceKey {
     return data == nullptr ? 0 : data[0];
   }
 
- protected:
-  ResourceKey() = default;
+  ResourceKey& operator=(const ResourceKey& that);
 
-  ResourceKey(uint32_t* data, size_t count);
+  bool operator==(const ResourceKey& that) const;
 
-  ResourceKey(const ResourceKey& that);
-
-  ResourceKey(ResourceKey&& key) noexcept;
+  bool operator!=(const ResourceKey& that) const {
+    return !(*this == that);
+  }
 
-  bool equal(const ResourceKey& that) const;
+ protected:
+  ResourceKey(uint32_t* data, size_t count);
 
-  void copy(const ResourceKey& that);
+  ResourceKey(ResourceKey&& key) noexcept;
 
   uint32_t* data = nullptr;
   size_t count = 0;
 };
 
+struct ResourceKeyHasher {
+  size_t operator()(const ResourceKey& key) const {
+    return key.hash();
+  }
+};
+
+template <typename T>
+using ResourceKeyMap = std::unordered_map<ResourceKey, T, ResourceKeyHasher>;
+
 /**
  * A key used for scratch resources. There are three important rules about scratch keys:
  *
@@ -92,33 +105,24 @@ class ScratchKey : public ResourceKey {
   }
 
   ScratchKey& operator=(const ScratchKey& that) {
-    copy(that);
+    ResourceKey::operator=(that);
     return *this;
   }
 
   ScratchKey& operator=(const BytesKey& that);
 
   bool operator==(const ScratchKey& that) const {
-    return equal(that);
+    return ResourceKey::operator==(that);
   }
 
   bool operator!=(const ScratchKey& that) const {
-    return !equal(that);
+    return !(*this == that);
   }
 
  private:
   ScratchKey(uint32_t* data, size_t count);
 };
 
-struct ScratchKeyHasher {
-  size_t operator()(const ScratchKey& scratchKey) const {
-    return scratchKey.hash();
-  }
-};
-
-template <typename T>
-using ScratchKeyMap = std::unordered_map<ScratchKey, T, ScratchKeyHasher>;
-
 class UniqueDomain;
 
 /**
@@ -184,11 +188,11 @@ class UniqueKey : public ResourceKey {
   UniqueKey& operator=(UniqueKey&& key) noexcept;
 
   bool operator==(const UniqueKey& that) const {
-    return equal(that);
+    return ResourceKey::operator==(that);
   }
 
   bool operator!=(const UniqueKey& that) const {
-    return !equal(that);
+    return !(*this == that);
   }
 
  private:
@@ -206,15 +210,6 @@ class UniqueKey : public ResourceKey {
   friend class LazyUniqueKey;
 };
 
-struct UniqueKeyHasher {
-  size_t operator()(const UniqueKey& uniqueKey) const {
-    return uniqueKey.hash();
-  }
-};
-
-template <typename T>
-using UniqueKeyMap = std::unordered_map<UniqueKey, T, UniqueKeyHasher>;
-
 /**
  * LazyUniqueKey defers the acquisition of the UniqueKey until it is actually needed.
  */

src/gpu/opengl/GLBuffer.cpp

@@ -22,14 +22,6 @@
 #include "core/utils/UniqueID.h"
 
 namespace tgfx {
-static ScratchKey ComputeScratchKey(BufferType bufferType) {
-  static const uint32_t Type = UniqueID::Next();
-  BytesKey bytesKey(2);
-  bytesKey.write(Type);
-  bytesKey.write(static_cast<uint32_t>(bufferType));
-  return bytesKey;
-}
-
 std::shared_ptr<GpuBuffer> GpuBuffer::Make(Context* context, const void* buffer, size_t size,
                                            BufferType bufferType) {
   if (buffer == nullptr || size == 0) {
@@ -39,19 +31,13 @@ std::shared_ptr<GpuBuffer> GpuBuffer::Make(Context* context, const void* buffer,
   CheckGLError(context);
 
   unsigned target = bufferType == BufferType::Index ? GL_ELEMENT_ARRAY_BUFFER : GL_ARRAY_BUFFER;
-  auto scratchKey = ComputeScratchKey(bufferType);
-  auto glBuffer = Resource::Find<GLBuffer>(context, scratchKey);
   auto gl = GLFunctions::Get(context);
-  if (glBuffer == nullptr) {
-    unsigned bufferID = 0;
-    gl->genBuffers(1, &bufferID);
-    if (bufferID == 0) {
-      return nullptr;
-    }
-    glBuffer = Resource::AddToCache(context, new GLBuffer(bufferType, size, bufferID), scratchKey);
-  } else {
-    glBuffer->_sizeInBytes = size;
+  unsigned bufferID = 0;
+  gl->genBuffers(1, &bufferID);
+  if (bufferID == 0) {
+    return nullptr;
   }
+  auto glBuffer = Resource::AddToCache(context, new GLBuffer(bufferType, size, bufferID));
   gl->bindBuffer(target, glBuffer->_bufferID);
   gl->bufferData(target, static_cast<GLsizeiptr>(size), buffer, GL_STATIC_DRAW);
   if (!CheckGLError(context)) {