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
diff --git a/libs/hwui/renderthread/VulkanManager.cpp b/libs/hwui/renderthread/VulkanManager.cpp
index 5a71833..f701491 100644
--- a/libs/hwui/renderthread/VulkanManager.cpp
+++ b/libs/hwui/renderthread/VulkanManager.cpp
@@ -425,27 +425,38 @@
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);
+ 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;
- 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();
+ 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 @@
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 @@
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 @@
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();