Merge "Fix some FD lifecycle issues" into sc-dev

This commit is contained in:
TreeHugger Robot
2021-05-14 01:13:25 +00:00
committed by Android (Google) Code Review
2 changed files with 21 additions and 22 deletions

View File

@@ -329,20 +329,16 @@ void VulkanSurface::releaseBuffers() {
if (bufferInfo.buffer.get() != nullptr && bufferInfo.dequeued) { if (bufferInfo.buffer.get() != nullptr && bufferInfo.dequeued) {
int err = mNativeWindow->cancelBuffer(mNativeWindow.get(), bufferInfo.buffer.get(), int err = mNativeWindow->cancelBuffer(mNativeWindow.get(), bufferInfo.buffer.get(),
bufferInfo.dequeue_fence); bufferInfo.dequeue_fence.release());
if (err != 0) { if (err != 0) {
ALOGE("cancelBuffer[%u] failed during destroy: %s (%d)", i, strerror(-err), err); ALOGE("cancelBuffer[%u] failed during destroy: %s (%d)", i, strerror(-err), err);
} }
bufferInfo.dequeued = false; bufferInfo.dequeued = false;
bufferInfo.dequeue_fence.reset();
if (bufferInfo.dequeue_fence >= 0) {
close(bufferInfo.dequeue_fence);
bufferInfo.dequeue_fence = -1;
}
} }
LOG_ALWAYS_FATAL_IF(bufferInfo.dequeued); LOG_ALWAYS_FATAL_IF(bufferInfo.dequeued);
LOG_ALWAYS_FATAL_IF(bufferInfo.dequeue_fence != -1); LOG_ALWAYS_FATAL_IF(bufferInfo.dequeue_fence.ok());
bufferInfo.skSurface.reset(); bufferInfo.skSurface.reset();
bufferInfo.buffer.clear(); bufferInfo.buffer.clear();
@@ -365,8 +361,12 @@ VulkanSurface::NativeBufferInfo* VulkanSurface::dequeueNativeBuffer() {
// Since auto pre-rotation is enabled, dequeueBuffer to get the consumer driven buffer size // Since auto pre-rotation is enabled, dequeueBuffer to get the consumer driven buffer size
// from ANativeWindowBuffer. // from ANativeWindowBuffer.
ANativeWindowBuffer* buffer; ANativeWindowBuffer* buffer;
int fence_fd; base::unique_fd fence_fd;
err = mNativeWindow->dequeueBuffer(mNativeWindow.get(), &buffer, &fence_fd); {
int rawFd = -1;
err = mNativeWindow->dequeueBuffer(mNativeWindow.get(), &buffer, &rawFd);
fence_fd.reset(rawFd);
}
if (err != 0) { if (err != 0) {
ALOGE("dequeueBuffer failed: %s (%d)", strerror(-err), err); ALOGE("dequeueBuffer failed: %s (%d)", strerror(-err), err);
return nullptr; return nullptr;
@@ -387,7 +387,7 @@ VulkanSurface::NativeBufferInfo* VulkanSurface::dequeueNativeBuffer() {
if (err != 0) { if (err != 0) {
ALOGE("native_window_set_buffers_transform(%d) failed: %s (%d)", transformHint, ALOGE("native_window_set_buffers_transform(%d) failed: %s (%d)", transformHint,
strerror(-err), err); strerror(-err), err);
mNativeWindow->cancelBuffer(mNativeWindow.get(), buffer, fence_fd); mNativeWindow->cancelBuffer(mNativeWindow.get(), buffer, fence_fd.release());
return nullptr; return nullptr;
} }
mWindowInfo.transform = transformHint; mWindowInfo.transform = transformHint;
@@ -405,19 +405,19 @@ VulkanSurface::NativeBufferInfo* VulkanSurface::dequeueNativeBuffer() {
for (idx = 0; idx < mWindowInfo.bufferCount; idx++) { for (idx = 0; idx < mWindowInfo.bufferCount; idx++) {
if (mNativeBuffers[idx].buffer.get() == buffer) { if (mNativeBuffers[idx].buffer.get() == buffer) {
mNativeBuffers[idx].dequeued = true; mNativeBuffers[idx].dequeued = true;
mNativeBuffers[idx].dequeue_fence = fence_fd; mNativeBuffers[idx].dequeue_fence = std::move(fence_fd);
break; break;
} else if (mNativeBuffers[idx].buffer.get() == nullptr) { } else if (mNativeBuffers[idx].buffer.get() == nullptr) {
// increasing the number of buffers we have allocated // increasing the number of buffers we have allocated
mNativeBuffers[idx].buffer = buffer; mNativeBuffers[idx].buffer = buffer;
mNativeBuffers[idx].dequeued = true; mNativeBuffers[idx].dequeued = true;
mNativeBuffers[idx].dequeue_fence = fence_fd; mNativeBuffers[idx].dequeue_fence = std::move(fence_fd);
break; break;
} }
} }
if (idx == mWindowInfo.bufferCount) { if (idx == mWindowInfo.bufferCount) {
ALOGE("dequeueBuffer returned unrecognized buffer"); ALOGE("dequeueBuffer returned unrecognized buffer");
mNativeWindow->cancelBuffer(mNativeWindow.get(), buffer, fence_fd); mNativeWindow->cancelBuffer(mNativeWindow.get(), buffer, fence_fd.release());
return nullptr; return nullptr;
} }
@@ -429,7 +429,7 @@ VulkanSurface::NativeBufferInfo* VulkanSurface::dequeueNativeBuffer() {
kTopLeft_GrSurfaceOrigin, mWindowInfo.colorspace, nullptr); kTopLeft_GrSurfaceOrigin, mWindowInfo.colorspace, nullptr);
if (bufferInfo->skSurface.get() == nullptr) { if (bufferInfo->skSurface.get() == nullptr) {
ALOGE("SkSurface::MakeFromAHardwareBuffer failed"); ALOGE("SkSurface::MakeFromAHardwareBuffer failed");
mNativeWindow->cancelBuffer(mNativeWindow.get(), buffer, fence_fd); mNativeWindow->cancelBuffer(mNativeWindow.get(), buffer, fence_fd.release());
return nullptr; return nullptr;
} }
} }
@@ -460,25 +460,23 @@ bool VulkanSurface::presentCurrentBuffer(const SkRect& dirtyRect, int semaphoreF
LOG_ALWAYS_FATAL_IF(!mCurrentBufferInfo); LOG_ALWAYS_FATAL_IF(!mCurrentBufferInfo);
VulkanSurface::NativeBufferInfo& currentBuffer = *mCurrentBufferInfo; VulkanSurface::NativeBufferInfo& currentBuffer = *mCurrentBufferInfo;
int queuedFd = (semaphoreFd != -1) ? semaphoreFd : currentBuffer.dequeue_fence; // queueBuffer always closes fence, even on error
int queuedFd = (semaphoreFd != -1) ? semaphoreFd : currentBuffer.dequeue_fence.release();
int err = mNativeWindow->queueBuffer(mNativeWindow.get(), currentBuffer.buffer.get(), queuedFd); int err = mNativeWindow->queueBuffer(mNativeWindow.get(), currentBuffer.buffer.get(), queuedFd);
currentBuffer.dequeued = false; currentBuffer.dequeued = false;
// queueBuffer always closes fence, even on error
if (err != 0) { if (err != 0) {
ALOGE("queueBuffer failed: %s (%d)", strerror(-err), err); ALOGE("queueBuffer failed: %s (%d)", strerror(-err), err);
// cancelBuffer takes ownership of the fence
mNativeWindow->cancelBuffer(mNativeWindow.get(), currentBuffer.buffer.get(), mNativeWindow->cancelBuffer(mNativeWindow.get(), currentBuffer.buffer.get(),
currentBuffer.dequeue_fence); currentBuffer.dequeue_fence.release());
} else { } else {
currentBuffer.hasValidContents = true; currentBuffer.hasValidContents = true;
currentBuffer.lastPresentedCount = mPresentCount; currentBuffer.lastPresentedCount = mPresentCount;
mPresentCount++; mPresentCount++;
} }
if (currentBuffer.dequeue_fence >= 0) { currentBuffer.dequeue_fence.reset();
close(currentBuffer.dequeue_fence);
currentBuffer.dequeue_fence = -1;
}
return err == 0; return err == 0;
} }

View File

@@ -15,6 +15,7 @@
*/ */
#pragma once #pragma once
#include <android-base/unique_fd.h>
#include <system/graphics.h> #include <system/graphics.h>
#include <system/window.h> #include <system/window.h>
#include <vulkan/vulkan.h> #include <vulkan/vulkan.h>
@@ -57,7 +58,7 @@ private:
// -1 any other time. When valid, we own the fd, and must ensure it is // -1 any other time. When valid, we own the fd, and must ensure it is
// closed: either by closing it explicitly when queueing the buffer, // closed: either by closing it explicitly when queueing the buffer,
// or by passing ownership e.g. to ANativeWindow::cancelBuffer(). // or by passing ownership e.g. to ANativeWindow::cancelBuffer().
int dequeue_fence = -1; base::unique_fd dequeue_fence;
bool dequeued = false; bool dequeued = false;
uint32_t lastPresentedCount = 0; uint32_t lastPresentedCount = 0;
bool hasValidContents = false; bool hasValidContents = false;