From 9fc3d279aee8175e0875f0cbe13d7cdfe92bbfa3 Mon Sep 17 00:00:00 2001
From: John Reck <jreck@google.com>
Date: Mon, 1 May 2023 16:33:22 -0400
Subject: [PATCH] Have GrContext hold a strong ref to VulkanManager

VulkanManager is already ref counted and can be reused
across GrContext's thanks to being shared with
HardwareBitmapUploader.

So take advantage of this + the newly added skia API
for GrContext being destroyed to have GrContext hold a
strong ref to VulkanManager which in turn keeps the
VkDevice alive. If somehow we never recover from this
we'll just keep using the initial device when creating
the next Grcontext which should be fine. Memory usage
will be sub-optimal but that's better than crashing
probably.

Bug: 266626090
Test: unable to repro bug issue, so speculative fix + it didn't get
worse at least?

Change-Id: I2038dacea679c29fd098462d4cf9d298e1215fb2
---
 libs/hwui/renderthread/CacheManager.cpp  |  9 +++++++-
 libs/hwui/renderthread/VulkanManager.cpp | 26 ++++++++++++++++++------
 libs/hwui/renderthread/VulkanManager.h   |  3 ++-
 3 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/libs/hwui/renderthread/CacheManager.cpp b/libs/hwui/renderthread/CacheManager.cpp
index 7e9d44fbdbd11..c00a2707e0a25 100644
--- a/libs/hwui/renderthread/CacheManager.cpp
+++ b/libs/hwui/renderthread/CacheManager.cpp
@@ -29,6 +29,7 @@
 #include "Layer.h"
 #include "Properties.h"
 #include "RenderThread.h"
+#include "VulkanManager.h"
 #include "pipeline/skia/ATraceMemoryDump.h"
 #include "pipeline/skia/ShaderCache.h"
 #include "pipeline/skia/SkiaMemoryTracer.h"
@@ -182,8 +183,14 @@ void CacheManager::dumpMemoryUsage(String8& log, const RenderState* renderState)
     }
     log.appendFormat("Contexts: %zu (stopped = %zu)\n", mCanvasContexts.size(), stoppedContexts);
 
+    auto vkInstance = VulkanManager::peekInstance();
     if (!mGrContext) {
-        log.appendFormat("No GPU context.\n");
+        if (!vkInstance) {
+            log.appendFormat("No GPU context.\n");
+        } else {
+            log.appendFormat("No GrContext; however %d remaining Vulkan refs",
+                             vkInstance->getStrongCount() - 1);
+        }
         return;
     }
     std::vector<skiapipeline::ResourcePair> cpuResourceMap = {
diff --git a/libs/hwui/renderthread/VulkanManager.cpp b/libs/hwui/renderthread/VulkanManager.cpp
index 96bfc1061d4e7..4695c7539988c 100644
--- a/libs/hwui/renderthread/VulkanManager.cpp
+++ b/libs/hwui/renderthread/VulkanManager.cpp
@@ -106,11 +106,11 @@ GrVkGetProc VulkanManager::sSkiaGetProp = [](const char* proc_name, VkInstance i
 #define GET_INST_PROC(F) m##F = (PFN_vk##F)vkGetInstanceProcAddr(mInstance, "vk" #F)
 #define GET_DEV_PROC(F) m##F = (PFN_vk##F)vkGetDeviceProcAddr(mDevice, "vk" #F)
 
-sp<VulkanManager> VulkanManager::getInstance() {
-    // cache a weakptr to the context to enable a second thread to share the same vulkan state
-    static wp<VulkanManager> sWeakInstance = nullptr;
-    static std::mutex sLock;
+// cache a weakptr to the context to enable a second thread to share the same vulkan state
+static wp<VulkanManager> sWeakInstance = nullptr;
+static std::mutex sLock;
 
+sp<VulkanManager> VulkanManager::getInstance() {
     std::lock_guard _lock{sLock};
     sp<VulkanManager> vulkanManager = sWeakInstance.promote();
     if (!vulkanManager.get()) {
@@ -121,6 +121,11 @@ sp<VulkanManager> VulkanManager::getInstance() {
     return vulkanManager;
 }
 
+sp<VulkanManager> VulkanManager::peekInstance() {
+    std::lock_guard _lock{sLock};
+    return sWeakInstance.promote();
+}
+
 VulkanManager::~VulkanManager() {
     if (mDevice != VK_NULL_HANDLE) {
         mDeviceWaitIdle(mDevice);
@@ -403,9 +408,13 @@ void VulkanManager::initialize() {
     }
 }
 
-sk_sp<GrDirectContext> VulkanManager::createContext(const GrContextOptions& options,
-                                                    ContextType contextType) {
+static void onGrContextReleased(void* context) {
+    VulkanManager* manager = (VulkanManager*)context;
+    manager->decStrong((void*)onGrContextReleased);
+}
 
+sk_sp<GrDirectContext> VulkanManager::createContext(GrContextOptions& options,
+                                                    ContextType contextType) {
     GrVkBackendContext backendContext;
     backendContext.fInstance = mInstance;
     backendContext.fPhysicalDevice = mPhysicalDevice;
@@ -417,6 +426,11 @@ sk_sp<GrDirectContext> VulkanManager::createContext(const GrContextOptions& opti
     backendContext.fDeviceFeatures2 = &mPhysicalDeviceFeatures2;
     backendContext.fGetProc = sSkiaGetProp;
 
+    LOG_ALWAYS_FATAL_IF(options.fContextDeleteProc != nullptr, "Conflicting fContextDeleteProcs!");
+    this->incStrong((void*)onGrContextReleased);
+    options.fContextDeleteContext = this;
+    options.fContextDeleteProc = onGrContextReleased;
+
     return GrDirectContext::MakeVulkan(backendContext, options);
 }
 
diff --git a/libs/hwui/renderthread/VulkanManager.h b/libs/hwui/renderthread/VulkanManager.h
index c5196eeccea37..00a40c0c85c36 100644
--- a/libs/hwui/renderthread/VulkanManager.h
+++ b/libs/hwui/renderthread/VulkanManager.h
@@ -66,6 +66,7 @@ class RenderThread;
 class VulkanManager final : public RefBase {
 public:
     static sp<VulkanManager> getInstance();
+    static sp<VulkanManager> peekInstance();
 
     // Sets up the vulkan context that is shared amonst all clients of the VulkanManager. This must
     // be call once before use of the VulkanManager. Multiple calls after the first will simiply
@@ -109,7 +110,7 @@ public:
     };
 
     // returns a Skia graphic context used to draw content on the specified thread
-    sk_sp<GrDirectContext> createContext(const GrContextOptions& options,
+    sk_sp<GrDirectContext> createContext(GrContextOptions& options,
                                          ContextType contextType = ContextType::kRenderThread);
 
     uint32_t getDriverVersion() const { return mDriverVersion; }