From 632e9302b66f002bb244aa0354d8b91b16261488 Mon Sep 17 00:00:00 2001
From: baldurk <baldurk@baldurk.org>
Date: Tue, 20 Oct 2020 16:11:28 +0100
Subject: [PATCH] Cache whether chunks are from an allocator

* If chunks come from an allocator they can't be safely deleted because the
  allocator may have been reset and recorded over where these chunks were with
  other data. Fortunately we don't need to do anything to delete them, so
  storing the allocator status up front is sufficient.
---
 renderdoc/core/resource_manager.h      | 29 +++++++++++++++++++-------
 renderdoc/driver/d3d11/d3d11_manager.h |  2 +-
 renderdoc/serialise/serialiser.cpp     |  7 +++++--
 renderdoc/serialise/serialiser.h       | 11 +++++-----
 4 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/renderdoc/core/resource_manager.h b/renderdoc/core/resource_manager.h
index c30a8e009..cebca2e85 100644
--- a/renderdoc/core/resource_manager.h
+++ b/renderdoc/core/resource_manager.h
@@ -381,7 +381,7 @@ struct ResourceRecord
     if(!dataWritten)
     {
       for(auto it = m_Chunks.begin(); it != m_Chunks.end(); ++it)
-        recordlist[it->first] = it->second;
+        recordlist[it->id] = it->chunk;
     }
   }
 
@@ -395,7 +395,7 @@ struct ResourceRecord
     if(ID == 0)
       ID = GetID();
     LockChunks();
-    m_Chunks.push_back({ID, chunk});
+    m_Chunks.push_back(StoredChunk(ID, chunk));
     UnlockChunks();
   }
 
@@ -428,7 +428,7 @@ struct ResourceRecord
     other->LockChunks();
 
     for(auto it = other->m_Chunks.begin(); it != other->m_Chunks.end(); ++it)
-      AddChunk(it->second->Duplicate());
+      AddChunk(it->chunk->Duplicate());
 
     for(auto it = other->Parents.begin(); it != other->Parents.end(); ++it)
       AddParent(*it);
@@ -441,7 +441,7 @@ struct ResourceRecord
   {
     LockChunks();
     for(auto it = m_Chunks.begin(); it != m_Chunks.end(); ++it)
-      it->second->Delete();
+      it->chunk->Delete(it->fromAllocator != 0);
     m_Chunks.clear();
     UnlockChunks();
   }
@@ -449,13 +449,13 @@ struct ResourceRecord
   Chunk *GetLastChunk() const
   {
     RDCASSERT(HasChunks());
-    return m_Chunks.back().second;
+    return m_Chunks.back().chunk;
   }
 
   int64_t GetLastChunkID() const
   {
     RDCASSERT(HasChunks());
-    return m_Chunks.back().first;
+    return m_Chunks.back().id;
   }
 
   void PopChunk() { m_Chunks.pop_back(); }
@@ -509,7 +509,22 @@ protected:
     return Atomic::Inc64(&globalIDCounter);
   }
 
-  rdcarray<rdcpair<int64_t, Chunk *>> m_Chunks;
+  struct StoredChunk
+  {
+    StoredChunk(int64_t i, Chunk *c)
+    {
+      id = i;
+      // we store this here because by the time it comes to delete the chunks the allocator may have
+      // already been reset and the contents trashed.
+      fromAllocator = c->IsFromAllocator() ? 1 : 0;
+      chunk = c;
+    }
+    int64_t id : 63;
+    int64_t fromAllocator : 1;
+    Chunk *chunk;
+  };
+
+  rdcarray<StoredChunk> m_Chunks;
   Threading::CriticalSection *m_ChunkLock;
 
   std::map<ResourceId, FrameRefType> m_FrameRefs;
diff --git a/renderdoc/driver/d3d11/d3d11_manager.h b/renderdoc/driver/d3d11/d3d11_manager.h
index c94c4dab8..948fc269a 100644
--- a/renderdoc/driver/d3d11/d3d11_manager.h
+++ b/renderdoc/driver/d3d11/d3d11_manager.h
@@ -187,7 +187,7 @@ struct D3D11ResourceRecord : public ResourceRecord
     if(!dataWritten)
     {
       for(auto it = m_Chunks.begin(); it != m_Chunks.end(); ++it)
-        recordlist[it->first] = it->second;
+        recordlist[it->id] = it->chunk;
 
       for(int i = 0; i < NumSubResources; i++)
         SubResources[i]->Insert(recordlist);
diff --git a/renderdoc/serialise/serialiser.cpp b/renderdoc/serialise/serialiser.cpp
index 460a3109c..687859bab 100644
--- a/renderdoc/serialise/serialiser.cpp
+++ b/renderdoc/serialise/serialiser.cpp
@@ -1006,10 +1006,13 @@ Chunk *Chunk::Create(Serialiser<SerialiserMode::Writing> &ser, uint16_t chunkTyp
   ret->m_ChunkType = chunkType;
   ret->m_Data = data;
 
+  if(allocator == NULL)
+  {
 #if ENABLED(RDOC_DEVEL)
-  Atomic::Inc64(&m_LiveChunks);
-  Atomic::ExchAdd64(&m_TotalMem, int64_t(length));
+    Atomic::Inc64(&m_LiveChunks);
+    Atomic::ExchAdd64(&m_TotalMem, int64_t(length));
 #endif
+  }
 
   return ret;
 }
diff --git a/renderdoc/serialise/serialiser.h b/renderdoc/serialise/serialiser.h
index cb2615ac3..9d5f7a807 100644
--- a/renderdoc/serialise/serialiser.h
+++ b/renderdoc/serialise/serialiser.h
@@ -1537,8 +1537,7 @@ class Chunk
   Chunk(bool fromAllocator) : m_FromAllocator(fromAllocator) {}
   ~Chunk()
   {
-    if(!m_FromAllocator)
-      FreeAlignedBuffer(m_Data);
+    FreeAlignedBuffer(m_Data);
 
 #if ENABLED(RDOC_DEVEL)
     Atomic::Dec64(&m_LiveChunks);
@@ -1547,11 +1546,11 @@ class Chunk
   }
 
 public:
-  void Delete()
+  void Delete() { Delete(m_FromAllocator); }
+  bool IsFromAllocator() { return m_FromAllocator; }
+  void Delete(bool fromAllocator)
   {
-    if(m_FromAllocator)
-      this->~Chunk();
-    else
+    if(!fromAllocator)
       delete this;
   }