Buffer DestroyImpl check for buffer resource to avoid double free

Double free was happening if buffer.Destroy() was called right
before the buffer went out of scope since DestroyImpl wouldn't
check to see if the resource was already released.

Also refactor Destroy in Texture classes to match the pattern
of Buffer classes. Added validation of the texture object when
Destroy is called.

Bug: dawn:124, chromium:947323
Change-Id: I0e4a652ff5b86a151b4919c781c1dd385b4e3213
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/6060
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Natasha Lee <natlee@microsoft.com>

src/dawn_native/Buffer.cpp

@@ -235,8 +235,7 @@ namespace dawn_native {
         if (mState == BufferState::Mapped) {
             Unmap();
         }
-        DestroyImpl();
-        mState = BufferState::Destroyed;
+        DestroyInternal();
     }
 
     void BufferBase::Unmap() {
@@ -331,4 +330,11 @@ namespace dawn_native {
         return {};
     }
 
+    void BufferBase::DestroyInternal() {
+        if (mState != BufferState::Destroyed) {
+            DestroyImpl();
+        }
+        mState = BufferState::Destroyed;
+    }
+
 }  // namespace dawn_native

src/dawn_native/Buffer.h

@@ -70,6 +70,8 @@ namespace dawn_native {
                                   void* pointer,
                                   uint32_t dataLength);
 
+        void DestroyInternal();
+
       private:
         virtual MaybeError SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data);
         virtual void MapReadAsyncImpl(uint32_t serial) = 0;

src/dawn_native/Texture.cpp

@@ -376,14 +376,6 @@ namespace dawn_native {
         return {};
     }
 
-    MaybeError TextureBase::ValidateCanCreateTextureViewNow() const {
-        ASSERT(!IsError());
-        if (mState == TextureState::Destroyed) {
-            return DAWN_VALIDATION_ERROR("Destroyed texture used to create texture view");
-        }
-        return {};
-    }
-
     bool TextureBase::IsMultisampledTexture() const {
         ASSERT(!IsError());
         return mSampleCount > 1;
@@ -404,13 +396,26 @@ namespace dawn_native {
     }
 
     void TextureBase::Destroy() {
+        if (GetDevice()->ConsumedError(ValidateDestroy())) {
+            return;
+        }
+        ASSERT(!IsError());
+        DestroyInternal();
+    }
+
+    void TextureBase::DestroyImpl() {
+    }
+
+    void TextureBase::DestroyInternal() {
         if (mState == TextureState::OwnedInternal) {
             DestroyImpl();
         }
         mState = TextureState::Destroyed;
     }
 
-    void TextureBase::DestroyImpl() {
+    MaybeError TextureBase::ValidateDestroy() const {
+        DAWN_TRY(GetDevice()->ValidateObject(this));
+        return {};
     }
 
     // TextureViewBase

src/dawn_native/Texture.h

@@ -61,7 +61,6 @@ namespace dawn_native {
         TextureState GetTextureState() const;
 
         MaybeError ValidateCanUseInSubmitNow() const;
-        MaybeError ValidateCanCreateTextureViewNow() const;
 
         bool IsMultisampledTexture() const;
 
@@ -70,10 +69,14 @@ namespace dawn_native {
         TextureViewBase* CreateTextureView(const TextureViewDescriptor* descriptor);
         void Destroy();
 
+      protected:
+        void DestroyInternal();
+
       private:
         TextureBase(DeviceBase* device, ObjectBase::ErrorTag tag);
         virtual void DestroyImpl();
 
+        MaybeError ValidateDestroy() const;
         dawn::TextureDimension mDimension;
         dawn::TextureFormat mFormat;
         Extent3D mSize;

src/dawn_native/d3d12/BufferD3D12.cpp

@@ -105,7 +105,7 @@ namespace dawn_native { namespace d3d12 {
     }
 
     Buffer::~Buffer() {
-        DestroyImpl();
+        DestroyInternal();
     }
 
     uint32_t Buffer::GetD3D12Size() const {

src/dawn_native/d3d12/TextureD3D12.cpp

@@ -141,7 +141,7 @@ namespace dawn_native { namespace d3d12 {
     }
 
     Texture::~Texture() {
-        Destroy();
+        DestroyInternal();
     }
 
     void Texture::DestroyImpl() {

src/dawn_native/metal/BufferMTL.mm

@@ -31,7 +31,7 @@ namespace dawn_native { namespace metal {
     }
 
     Buffer::~Buffer() {
-        DestroyImpl();
+        DestroyInternal();
     }
 
     id<MTLBuffer> Buffer::GetMTLBuffer() {

src/dawn_native/metal/TextureMTL.mm

@@ -219,7 +219,7 @@ namespace dawn_native { namespace metal {
     }
 
     Texture::~Texture() {
-        Destroy();
+        DestroyInternal();
     }
 
     void Texture::DestroyImpl() {

src/dawn_native/opengl/BufferGL.cpp

@@ -28,7 +28,7 @@ namespace dawn_native { namespace opengl {
     }
 
     Buffer::~Buffer() {
-        DestroyImpl();
+        DestroyInternal();
     }
 
     GLuint Buffer::GetHandle() const {

src/dawn_native/opengl/TextureGL.cpp

@@ -177,7 +177,7 @@ namespace dawn_native { namespace opengl {
     }
 
     Texture::~Texture() {
-        Destroy();
+        DestroyInternal();
     }
 
     void Texture::DestroyImpl() {

src/dawn_native/vulkan/BufferVk.cpp

@@ -137,7 +137,7 @@ namespace dawn_native { namespace vulkan {
     }
 
     Buffer::~Buffer() {
-        DestroyImpl();
+        DestroyInternal();
     }
 
     void Buffer::OnMapReadCommandSerialFinished(uint32_t mapSerial, const void* data) {

src/dawn_native/vulkan/TextureVk.cpp

@@ -297,7 +297,7 @@ namespace dawn_native { namespace vulkan {
     }
 
     Texture::~Texture() {
-        Destroy();
+        DestroyInternal();
     }
 
     void Texture::DestroyImpl() {