From bc5f06bdaa95da7e2a94d8827fffbadad7bc813d Mon Sep 17 00:00:00 2001
From: Stan Iliev <stani@google.com>
Date: Tue, 26 Mar 2019 15:14:34 -0400
Subject: [PATCH] Fix crash after dequeueNativeBuffer fails

Anytime dequeueNativeBuffer fails, it sets mDequeuedIndex to -1.
This is causing a crash latter, when getCurrentSkSurface tries
to index mNativeBuffers[mDequeuedIndex].
This CL removes mDequeuedIndex and improves Vulkan swapchain
error handling.

Test: Ran Camera app
Bug: 129024275
Change-Id: Ieeb685c3a1b33f23ce2334d286199a44ace53165
---
 libs/hwui/renderthread/VulkanManager.cpp |  8 ++++++--
 libs/hwui/renderthread/VulkanSurface.cpp | 13 ++++++++-----
 libs/hwui/renderthread/VulkanSurface.h   |  8 +++++---
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/libs/hwui/renderthread/VulkanManager.cpp b/libs/hwui/renderthread/VulkanManager.cpp
index b8ebf3bb0ca91..c929098986528 100644
--- a/libs/hwui/renderthread/VulkanManager.cpp
+++ b/libs/hwui/renderthread/VulkanManager.cpp
@@ -493,6 +493,12 @@ void VulkanManager::swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect)
         mDeviceWaitIdle(mDevice);
     }
 
+    VulkanSurface::NativeBufferInfo* bufferInfo = surface->getCurrentBufferInfo();
+    if (!bufferInfo) {
+        // If VulkanSurface::dequeueNativeBuffer failed earlier, then swapBuffers is a no-op.
+        return;
+    }
+
     VkExportSemaphoreCreateInfo exportInfo;
     exportInfo.sType = VK_STRUCTURE_TYPE_EXPORT_SEMAPHORE_CREATE_INFO;
     exportInfo.pNext = nullptr;
@@ -509,8 +515,6 @@ void VulkanManager::swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect)
     GrBackendSemaphore backendSemaphore;
     backendSemaphore.initVulkan(semaphore);
 
-    VulkanSurface::NativeBufferInfo* bufferInfo = surface->getCurrentBufferInfo();
-
     int fenceFd = -1;
     GrSemaphoresSubmitted submitted =
             bufferInfo->skSurface->flush(SkSurface::BackendSurfaceAccess::kPresent,
diff --git a/libs/hwui/renderthread/VulkanSurface.cpp b/libs/hwui/renderthread/VulkanSurface.cpp
index a98eb322cfc76..1708d3c0e0936 100644
--- a/libs/hwui/renderthread/VulkanSurface.cpp
+++ b/libs/hwui/renderthread/VulkanSurface.cpp
@@ -420,9 +420,10 @@ void VulkanSurface::releaseBuffers() {
 }
 
 VulkanSurface::NativeBufferInfo* VulkanSurface::dequeueNativeBuffer() {
-    // Set the dequeue index to invalid in case of error and only reset it to the correct
+    // Set the mCurrentBufferInfo to invalid in case of error and only reset it to the correct
     // value at the end of the function if everything dequeued correctly.
-    mDequeuedIndex = -1;
+    mCurrentBufferInfo = nullptr;
+
 
     //check if the native window has been resized or rotated and update accordingly
     SkISize newSize = SkISize::MakeEmpty();
@@ -511,7 +512,7 @@ VulkanSurface::NativeBufferInfo* VulkanSurface::dequeueNativeBuffer() {
         }
     }
 
-    mDequeuedIndex = idx;
+    mCurrentBufferInfo = bufferInfo;
     return bufferInfo;
 }
 
@@ -535,7 +536,8 @@ bool VulkanSurface::presentCurrentBuffer(const SkRect& dirtyRect, int semaphoreF
         ALOGE_IF(err != 0, "native_window_set_surface_damage failed: %s (%d)", strerror(-err), err);
     }
 
-    VulkanSurface::NativeBufferInfo& currentBuffer = mNativeBuffers[mDequeuedIndex];
+    LOG_ALWAYS_FATAL_IF(!mCurrentBufferInfo);
+    VulkanSurface::NativeBufferInfo& currentBuffer = *mCurrentBufferInfo;
     int queuedFd = (semaphoreFd != -1) ? semaphoreFd : currentBuffer.dequeue_fence;
     int err = mNativeWindow->queueBuffer(mNativeWindow.get(), currentBuffer.buffer.get(), queuedFd);
 
@@ -560,7 +562,8 @@ bool VulkanSurface::presentCurrentBuffer(const SkRect& dirtyRect, int semaphoreF
 }
 
 int VulkanSurface::getCurrentBuffersAge() {
-    VulkanSurface::NativeBufferInfo& currentBuffer = mNativeBuffers[mDequeuedIndex];
+    LOG_ALWAYS_FATAL_IF(!mCurrentBufferInfo);
+    VulkanSurface::NativeBufferInfo& currentBuffer = *mCurrentBufferInfo;
     return currentBuffer.hasValidContents ? (mPresentCount - currentBuffer.lastPresentedCount) : 0;
 }
 
diff --git a/libs/hwui/renderthread/VulkanSurface.h b/libs/hwui/renderthread/VulkanSurface.h
index 4fd9cd29b9201..418d40f8c056e 100644
--- a/libs/hwui/renderthread/VulkanSurface.h
+++ b/libs/hwui/renderthread/VulkanSurface.h
@@ -42,7 +42,9 @@ public:
                                   const VulkanManager& vkManager);
     ~VulkanSurface();
 
-    sk_sp<SkSurface> getCurrentSkSurface() { return mNativeBuffers[mDequeuedIndex].skSurface; }
+    sk_sp<SkSurface> getCurrentSkSurface() {
+        return mCurrentBufferInfo ? mCurrentBufferInfo->skSurface : nullptr;
+    }
     const SkMatrix& getCurrentPreTransform() { return mWindowInfo.preTransform; }
 
 private:
@@ -65,7 +67,7 @@ private:
     };
 
     NativeBufferInfo* dequeueNativeBuffer();
-    NativeBufferInfo* getCurrentBufferInfo() { return &mNativeBuffers[mDequeuedIndex]; }
+    NativeBufferInfo* getCurrentBufferInfo() { return mCurrentBufferInfo; }
     bool presentCurrentBuffer(const SkRect& dirtyRect, int semaphoreFd);
 
     // The width and height are are the logical width and height for when submitting draws to the
@@ -115,8 +117,8 @@ private:
     WindowInfo mWindowInfo;
     GrContext* mGrContext;
 
-    int mDequeuedIndex = -1;
     uint32_t mPresentCount = 0;
+    NativeBufferInfo* mCurrentBufferInfo = nullptr;
 
     const SkISize mMinWindowSize;
     const SkISize mMaxWindowSize;