commit fd4293962c243871d95fb6bb6a08d6a20a8e8854
author Greg Daniel <egdaniel@google.com> Thu May 09 15:44:56 2019 -0400
committer Greg Daniel <egdaniel@google.com> Wed May 15 14:19:16 2019 -0400
tree 288698458439e68a613358c7655a85a2d9d46af8
parent dac8ddd73ea74d0fc18d8f798ac582cecda4897f

Make sure we don't delete VkSemaphores before exporting in VulkanManager

Test: Manual building and testing on device to confirm error messages
are gone.

Bug: b/132358913
Change-Id: I0e22df5eb3bc61a7dd84d87db9a4f67756ecd5ae

libs/hwui/renderthread/VulkanManager.cpp

diff --git a/libs/hwui/renderthread/VulkanManager.cpp b/libs/hwui/renderthread/VulkanManager.cpp
index 62fd489..ce5be8a 100644
--- a/libs/hwui/renderthread/VulkanManager.cpp
+++ b/libs/hwui/renderthread/VulkanManager.cpp
@@ -482,6 +482,13 @@
     PFN_vkDestroySemaphore mDestroyFunction;
     VkDevice mDevice;
     VkSemaphore mSemaphore;
+    // We need to make sure we don't delete the VkSemaphore until it is done being used by both Skia
+    // (including by the GPU) and inside the VulkanManager. So we always start with two refs, one
+    // owned by Skia and one owned by the VulkanManager. The refs are decremented each time
+    // destroy_semaphore is called with this object. Skia will call destroy_semaphore once it is
+    // done with the semaphore and the GPU has finished work on the semaphore. The VulkanManager
+    // calls destroy_semaphore after sending the semaphore to Skia and exporting it if need be.
+    int mRefs = 2;
 
     DestroySemaphoreInfo(PFN_vkDestroySemaphore destroyFunction, VkDevice device,
             VkSemaphore semaphore)
@@ -490,8 +497,11 @@
 
 static void destroy_semaphore(void* context) {
     DestroySemaphoreInfo* info = reinterpret_cast<DestroySemaphoreInfo*>(context);
-    info->mDestroyFunction(info->mDevice, info->mSemaphore, nullptr);
-    delete info;
+    --info->mRefs;
+    if (!info->mRefs) {
+        info->mDestroyFunction(info->mDevice, info->mSemaphore, nullptr);
+        delete info;
+    }
 }
 
 void VulkanManager::swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect) {
@@ -542,6 +552,7 @@
         ALOGE("VulkanManager::swapBuffers(): Semaphore submission failed");
         mQueueWaitIdle(mGraphicsQueue);
     }
+    destroy_semaphore(destroyInfo);
 
     surface->presentCurrentBuffer(dirtyRect, fenceFd);
 }
@@ -644,13 +655,16 @@
 
     DestroySemaphoreInfo* destroyInfo = new DestroySemaphoreInfo(mDestroySemaphore, mDevice,
                                                                  semaphore);
+    // Even if Skia fails to submit the semaphore, it will still call the destroy_semaphore callback
+    // which will remove its ref to the semaphore. The VulkanManager must still release its ref,
+    // when it is done with the semaphore.
     GrSemaphoresSubmitted submitted =
             grContext->flush(kNone_GrFlushFlags, 1, &backendSemaphore,
                              destroy_semaphore, destroyInfo);
 
     if (submitted == GrSemaphoresSubmitted::kNo) {
         ALOGE("VulkanManager::createReleaseFence: Failed to submit semaphore");
-        mDestroySemaphore(mDevice, semaphore, nullptr);
+        destroy_semaphore(destroyInfo);
         return INVALID_OPERATION;
     }
 
@@ -663,6 +677,7 @@
     int fenceFd = 0;
 
     err = mGetSemaphoreFdKHR(mDevice, &getFdInfo, &fenceFd);
+    destroy_semaphore(destroyInfo);
     if (VK_SUCCESS != err) {
         ALOGE("VulkanManager::createReleaseFence: Failed to get semaphore Fd");
         return INVALID_OPERATION;