From df287cbffdddce3c753e6182d4cf7bfd43ba1b63 Mon Sep 17 00:00:00 2001
From: David Reveman <reveman@google.com>
Date: Fri, 12 Feb 2021 09:18:54 -0500
Subject: [PATCH] Fix vulkan surface leaks.

This fixes 3 memory leaks:

1. Destroys local vulkan buffer collections.
2. Releases image2 resource in Scenic.
3. Deregister buffer collections from Scenic session.
---
 .../fuchsia/flutter/vulkan_surface.cc         | 29 ++++++++++++++-----
 .../platform/fuchsia/flutter/vulkan_surface.h |  7 +++--
 vulkan/vulkan_proc_table.cc                   |  1 +
 vulkan/vulkan_proc_table.h                    |  1 +
 4 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/shell/platform/fuchsia/flutter/vulkan_surface.cc b/shell/platform/fuchsia/flutter/vulkan_surface.cc
index f36fdea225628..c5f47ed75b571 100644
--- a/shell/platform/fuchsia/flutter/vulkan_surface.cc
+++ b/shell/platform/fuchsia/flutter/vulkan_surface.cc
@@ -107,13 +107,11 @@ VulkanSurface::VulkanSurface(
     scenic::Session* session,
     const SkISize& size,
     uint32_t buffer_id)
-    : vulkan_provider_(vulkan_provider),
-      session_(session),
-      buffer_id_(buffer_id),
-      wait_(this) {
+    : vulkan_provider_(vulkan_provider), session_(session), wait_(this) {
   FML_DCHECK(session_);
 
-  if (!AllocateDeviceMemory(sysmem_allocator, std::move(context), size)) {
+  if (!AllocateDeviceMemory(sysmem_allocator, std::move(context), size,
+                            buffer_id)) {
     FML_DLOG(INFO) << "Could not allocate device memory.";
     return;
   }
@@ -135,6 +133,12 @@ VulkanSurface::VulkanSurface(
 }
 
 VulkanSurface::~VulkanSurface() {
+  if (image_id_) {
+    session_->Enqueue(scenic::NewReleaseResourceCmd(image_id_));
+  }
+  if (buffer_id_) {
+    session_->DeregisterBufferCollection(buffer_id_);
+  }
   wait_.Cancel();
   wait_.set_object(ZX_HANDLE_INVALID);
 }
@@ -221,7 +225,8 @@ bool VulkanSurface::CreateFences() {
 bool VulkanSurface::AllocateDeviceMemory(
     fuchsia::sysmem::AllocatorSyncPtr& sysmem_allocator,
     sk_sp<GrDirectContext> context,
-    const SkISize& size) {
+    const SkISize& size,
+    uint32_t buffer_id) {
   if (size.isEmpty()) {
     return false;
   }
@@ -237,16 +242,24 @@ bool VulkanSurface::AllocateDeviceMemory(
   status = vulkan_token->Sync();
   LOG_AND_RETURN(status != ZX_OK, "Failed to sync token");
 
-  session_->RegisterBufferCollection(buffer_id_, std::move(scenic_token));
+  session_->RegisterBufferCollection(buffer_id, std::move(scenic_token));
+  buffer_id_ = buffer_id;
 
   VkBufferCollectionCreateInfoFUCHSIA import_info;
   import_info.collectionToken = vulkan_token.Unbind().TakeChannel().release();
+  VkBufferCollectionFUCHSIA collection;
   if (VK_CALL_LOG_ERROR(vulkan_provider_.vk().CreateBufferCollectionFUCHSIA(
-          vulkan_provider_.vk_device(), &import_info, nullptr, &collection_)) !=
+          vulkan_provider_.vk_device(), &import_info, nullptr, &collection)) !=
       VK_SUCCESS) {
     return false;
   }
 
+  collection_ = {collection, [&vulkan_provider = vulkan_provider_](
+                                 VkBufferCollectionFUCHSIA collection) {
+                   vulkan_provider.vk().DestroyBufferCollectionFUCHSIA(
+                       vulkan_provider.vk_device(), collection, nullptr);
+                 }};
+
   VulkanImage vulkan_image;
   LOG_AND_RETURN(!CreateVulkanImage(vulkan_provider_, size, &vulkan_image),
                  "Failed to create VkImage");
diff --git a/shell/platform/fuchsia/flutter/vulkan_surface.h b/shell/platform/fuchsia/flutter/vulkan_surface.h
index 45ad52a45f5b2..7e54bd23688df 100644
--- a/shell/platform/fuchsia/flutter/vulkan_surface.h
+++ b/shell/platform/fuchsia/flutter/vulkan_surface.h
@@ -146,7 +146,8 @@ class VulkanSurface final : public SurfaceProducerSurface {
 
   bool AllocateDeviceMemory(fuchsia::sysmem::AllocatorSyncPtr& sysmem_allocator,
                             sk_sp<GrDirectContext> context,
-                            const SkISize& size);
+                            const SkISize& size,
+                            uint32_t buffer_id);
 
   bool CreateVulkanImage(vulkan::VulkanProvider& vulkan_provider,
                          const SkISize& size,
@@ -174,9 +175,9 @@ class VulkanSurface final : public SurfaceProducerSurface {
   VkMemoryAllocateInfo vk_memory_info_;
   vulkan::VulkanHandle<VkFence> command_buffer_fence_;
   sk_sp<SkSurface> sk_surface_;
-  const uint32_t buffer_id_;
+  uint32_t buffer_id_ = 0;
   uint32_t image_id_ = 0;
-  VkBufferCollectionFUCHSIA collection_;
+  vulkan::VulkanHandle<VkBufferCollectionFUCHSIA> collection_;
   zx::event acquire_event_;
   vulkan::VulkanHandle<VkSemaphore> acquire_semaphore_;
   std::unique_ptr<vulkan::VulkanCommandBuffer> command_buffer_;
diff --git a/vulkan/vulkan_proc_table.cc b/vulkan/vulkan_proc_table.cc
index b4d4bc86e23ca..6bd2c9730036d 100644
--- a/vulkan/vulkan_proc_table.cc
+++ b/vulkan/vulkan_proc_table.cc
@@ -138,6 +138,7 @@ bool VulkanProcTable::SetupDeviceProcAddresses(
 #endif  // OS_ANDROID
 #if OS_FUCHSIA
   ACQUIRE_PROC(CreateBufferCollectionFUCHSIA, handle);
+  ACQUIRE_PROC(DestroyBufferCollectionFUCHSIA, handle);
   ACQUIRE_PROC(GetMemoryZirconHandleFUCHSIA, handle);
   ACQUIRE_PROC(ImportSemaphoreZirconHandleFUCHSIA, handle);
   ACQUIRE_PROC(SetBufferCollectionConstraintsFUCHSIA, handle);
diff --git a/vulkan/vulkan_proc_table.h b/vulkan/vulkan_proc_table.h
index 117103c02f503..281385440b527 100644
--- a/vulkan/vulkan_proc_table.h
+++ b/vulkan/vulkan_proc_table.h
@@ -116,6 +116,7 @@ class VulkanProcTable : public fml::RefCountedThreadSafe<VulkanProcTable> {
 #endif  // OS_ANDROID
 #if OS_FUCHSIA
   DEFINE_PROC(CreateBufferCollectionFUCHSIA);
+  DEFINE_PROC(DestroyBufferCollectionFUCHSIA);
   DEFINE_PROC(GetMemoryZirconHandleFUCHSIA);
   DEFINE_PROC(ImportSemaphoreZirconHandleFUCHSIA);
   DEFINE_PROC(SetBufferCollectionConstraintsFUCHSIA);