From d667077899b2e4c9deb3485fdbcdc5f1e3bb5e37 Mon Sep 17 00:00:00 2001
From: Greg Daniel <egdaniel@google.com>
Date: Wed, 9 Jun 2021 12:01:12 -0400
Subject: Make sure to close vulkan fd's in error cases

In one set of the fixed cases we actually were never closing the
fd. In the other set we would crash with a fatal error. Now both
have been updated to handle the failures and correctly close the
fd to make sure it all works.

However, I don't think this is the route case of the leaking fds
from the bug since I don't believe we are seeing any of the
corresponding error messages in the logs.

Test: Manual running on phone and watching logs.
Bug: 187240173
Change-Id: I18babe91b0ec83ca63199f06d3d02ec10e8aea85
---
 libs/hwui/renderthread/VulkanManager.cpp | 58 ++++++++++++++++++++------------
 1 file changed, 36 insertions(+), 22 deletions(-)

(limited to 'libs/hwui/renderthread/VulkanManager.cpp')

diff --git a/libs/hwui/renderthread/VulkanManager.cpp b/libs/hwui/renderthread/VulkanManager.cpp
index 5a718330d68f..f70149111116 100644
--- a/libs/hwui/renderthread/VulkanManager.cpp
+++ b/libs/hwui/renderthread/VulkanManager.cpp
@@ -425,27 +425,38 @@ Frame VulkanManager::dequeueNextBuffer(VulkanSurface* surface) {
                 semaphoreInfo.flags = 0;
                 VkSemaphore semaphore;
                 VkResult err = mCreateSemaphore(mDevice, &semaphoreInfo, nullptr, &semaphore);
-                LOG_ALWAYS_FATAL_IF(VK_SUCCESS != err, "Failed to create import semaphore, err: %d",
-                                    err);
-
-                VkImportSemaphoreFdInfoKHR importInfo;
-                importInfo.sType = VK_STRUCTURE_TYPE_IMPORT_SEMAPHORE_FD_INFO_KHR;
-                importInfo.pNext = nullptr;
-                importInfo.semaphore = semaphore;
-                importInfo.flags = VK_SEMAPHORE_IMPORT_TEMPORARY_BIT;
-                importInfo.handleType = VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT;
-                importInfo.fd = fence_clone;
-
-                err = mImportSemaphoreFdKHR(mDevice, &importInfo);
-                LOG_ALWAYS_FATAL_IF(VK_SUCCESS != err, "Failed to import semaphore, err: %d", err);
-
-                GrBackendSemaphore backendSemaphore;
-                backendSemaphore.initVulkan(semaphore);
-                bufferInfo->skSurface->wait(1, &backendSemaphore);
-                // The following flush blocks the GPU immediately instead of waiting for other
-                // drawing ops. It seems dequeue_fence is not respected otherwise.
-                // TODO: remove the flush after finding why backendSemaphore is not working.
-                bufferInfo->skSurface->flushAndSubmit();
+                if (err != VK_SUCCESS) {
+                    ALOGE("Failed to create import semaphore, err: %d", err);
+                    close(fence_clone);
+                    sync_wait(bufferInfo->dequeue_fence, -1 /* forever */);
+                } else {
+                    VkImportSemaphoreFdInfoKHR importInfo;
+                    importInfo.sType = VK_STRUCTURE_TYPE_IMPORT_SEMAPHORE_FD_INFO_KHR;
+                    importInfo.pNext = nullptr;
+                    importInfo.semaphore = semaphore;
+                    importInfo.flags = VK_SEMAPHORE_IMPORT_TEMPORARY_BIT;
+                    importInfo.handleType = VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT;
+                    importInfo.fd = fence_clone;
+
+                    err = mImportSemaphoreFdKHR(mDevice, &importInfo);
+                    if (err != VK_SUCCESS) {
+                        ALOGE("Failed to import semaphore, err: %d", err);
+                        mDestroySemaphore(mDevice, semaphore, nullptr);
+                        close(fence_clone);
+                        sync_wait(bufferInfo->dequeue_fence, -1 /* forever */);
+                    } else {
+                        GrBackendSemaphore backendSemaphore;
+                        backendSemaphore.initVulkan(semaphore);
+                        // Skia will take ownership of the VkSemaphore and delete it once the wait
+                        // has finished. The VkSemaphore also owns the imported fd, so it will
+                        // close the fd when it is deleted.
+                        bufferInfo->skSurface->wait(1, &backendSemaphore);
+                        // The following flush blocks the GPU immediately instead of waiting for
+                        // other drawing ops. It seems dequeue_fence is not respected otherwise.
+                        // TODO: remove the flush after finding why backendSemaphore is not working.
+                        bufferInfo->skSurface->flushAndSubmit();
+                    }
+                }
             }
         }
     }
@@ -621,6 +632,7 @@ status_t VulkanManager::fenceWait(int fence, GrDirectContext* grContext) {
     VkSemaphore semaphore;
     VkResult err = mCreateSemaphore(mDevice, &semaphoreInfo, nullptr, &semaphore);
     if (VK_SUCCESS != err) {
+        close(fenceFd);
         ALOGE("Failed to create import semaphore, err: %d", err);
         return UNKNOWN_ERROR;
     }
@@ -635,6 +647,7 @@ status_t VulkanManager::fenceWait(int fence, GrDirectContext* grContext) {
     err = mImportSemaphoreFdKHR(mDevice, &importInfo);
     if (VK_SUCCESS != err) {
         mDestroySemaphore(mDevice, semaphore, nullptr);
+        close(fenceFd);
         ALOGE("Failed to import semaphore, err: %d", err);
         return UNKNOWN_ERROR;
     }
@@ -642,7 +655,8 @@ status_t VulkanManager::fenceWait(int fence, GrDirectContext* grContext) {
     GrBackendSemaphore beSemaphore;
     beSemaphore.initVulkan(semaphore);
 
-    // Skia takes ownership of the semaphore and will delete it once the wait has finished.
+    // Skia will take ownership of the VkSemaphore and delete it once the wait has finished. The
+    // VkSemaphore also owns the imported fd, so it will close the fd when it is deleted.
     grContext->wait(1, &beSemaphore);
     grContext->flushAndSubmit();
 
-- 
cgit v1.2.3