commit 91a88f0715ac08cdf2328681d4740f59a4d80fe4
author Brian Salomon <bsalomon@google.com> Thu Feb 04 15:34:32 2021 -0500
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Fri Feb 05 02:13:39 2021 +0000
tree d476dd0bf3274b11b9c65f66f4ca95c295fa7388
parent 9f704a86400866381cf423a46e876f0c70af3167

Keep mapped buffer manager alive until after buffers are disposed of.

There is currently window of time where we have removed the Inbox
that receives messages about buffers that need to be unmapped and
the disposal (abandonment/releasing) by GrDirectContext.

If on another thread an async read result is released it will attempt
to post to the inbox and transfer its buffer ref along with the
posted message. However, the inbox is gone so it unrefs the
buffer and we delete the buffer on that thread instead of in
~GrDirectContext.


Speculative fix for:

Bug: chromium:1153592
Change-Id: I223bcedca624faa159b0f4c6fcb9e45aded64dff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366403
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

src/gpu/GrDirectContext.cpp

diff --git a/src/gpu/GrDirectContext.cpp b/src/gpu/GrDirectContext.cpp
index 6e393e7..cdad131 100644
--- a/src/gpu/GrDirectContext.cpp
+++ b/src/gpu/GrDirectContext.cpp
@@ -67,12 +67,14 @@
     this->syncAllOutstandingGpuWork(/*shouldExecuteWhileAbandoned=*/false);
 
     this->destroyDrawingManager();
-    fMappedBufferManager.reset();
 
     // Ideally we could just let the ptr drop, but resource cache queries this ptr in releaseAll.
     if (fResourceCache) {
         fResourceCache->releaseAll();
     }
+    // This has to be after GrResourceCache::releaseAll so that other threads that are holding
+    // async pixel result don't try to destroy buffers off thread.
+    fMappedBufferManager.reset();
 }
 
 sk_sp<GrContextThreadSafeProxy> GrDirectContext::threadSafeProxy() {
@@ -112,7 +114,9 @@
 
     fGpu->disconnect(GrGpu::DisconnectType::kAbandon);
 
+    // Must be after GrResourceCache::abandonAll().
     fMappedBufferManager.reset();
+
     if (fSmallPathAtlasMgr) {
         fSmallPathAtlasMgr->reset();
     }
@@ -143,13 +147,14 @@
     // We need to make sure all work is finished on the gpu before we start releasing resources.
     this->syncAllOutstandingGpuWork(/*shouldExecuteWhileAbandoned=*/true);
 
-    fMappedBufferManager.reset();
-
     fResourceProvider->abandon();
 
     // Release all resources in the backend 3D API.
     fResourceCache->releaseAll();
 
+    // Must be after GrResourceCache::releaseAll().
+    fMappedBufferManager.reset();
+
     fGpu->disconnect(GrGpu::DisconnectType::kCleanup);
     if (fSmallPathAtlasMgr) {
         fSmallPathAtlasMgr->reset();