Make sure GrResourceCache frees resources waiting on messages during destruction.

Bug: skia:6812
Change-Id: I86c50a9e301ae4846e638c915f94b797d60ee9a1
Reviewed-on: https://skia-review.googlesource.com/146646
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This commit is contained in:
Greg Daniel 2018-08-10 09:48:08 -04:00 committed by Skia Commit-Bot
parent 93ce79dba4
commit c27eb726bd
4 changed files with 56 additions and 2 deletions

View File

@ -243,8 +243,6 @@ sk_sp<GrTextureProxy> GrAHardwareBufferImageGenerator::makeProxy(GrContext* cont
// in the correct thread/context. This adds the only ref to the texture that will persist from
// this point. To trigger GrTexture deletion a message is sent by generator dtor or by
// makeProxy when it is invoked with a different context.
//TODO: GrResourceCache should delete GrTexture, when GrContext is deleted. Currently
//TODO: SkMessageBus ignores messages for deleted contexts and GrTexture will leak.
context->contextPriv().getResourceCache()->insertCrossContextGpuResource(fOriginalTexture);
return proxyProvider->createWrapped(std::move(tex), kTopLeft_GrSurfaceOrigin);
}

View File

@ -200,6 +200,12 @@ void GrResourceCache::releaseAll() {
this->processFreedGpuResources();
// We need to make sure to free any resources that were waiting on a free message but never
// received one.
for (int i = 0; i < fResourcesWaitingForFreeMsg.count(); ++i) {
fResourcesWaitingForFreeMsg[i]->unref();
}
SkASSERT(fProxyProvider); // better have called setProxyProvider
// We must remove the uniqueKeys from the proxies here. While they possess a uniqueKey
// they also have a raw pointer back to this class (which is presumably going away)!
@ -600,6 +606,8 @@ void GrResourceCache::processInvalidUniqueKeys(
void GrResourceCache::insertCrossContextGpuResource(GrGpuResource* resource) {
resource->ref();
SkASSERT(!fResourcesWaitingForFreeMsg.contains(resource));
fResourcesWaitingForFreeMsg.push_back(resource);
}
void GrResourceCache::processFreedGpuResources() {
@ -607,6 +615,9 @@ void GrResourceCache::processFreedGpuResources() {
fFreedGpuResourceInbox.poll(&msgs);
for (int i = 0; i < msgs.count(); ++i) {
SkASSERT(msgs[i].fOwningUniqueID == fContextUniqueID);
int index = fResourcesWaitingForFreeMsg.find(msgs[i].fResource);
SkASSERT(index != -1);
fResourcesWaitingForFreeMsg.removeShuffle(index);
msgs[i].fResource->unref();
}
}

View File

@ -371,6 +371,8 @@ private:
InvalidUniqueKeyInbox fInvalidUniqueKeyInbox;
FreedGpuResourceInbox fFreedGpuResourceInbox;
SkTDArray<GrGpuResource*> fResourcesWaitingForFreeMsg;
uint32_t fContextUniqueID;
// This resource is allowed to be in the nonpurgeable array for the sake of validate() because

View File

@ -360,6 +360,10 @@ public:
GrContext* context() { return fContext.get(); }
void reset() {
fContext.reset();
}
private:
sk_sp<GrContext> fContext;
};
@ -1595,6 +1599,44 @@ static void test_tags(skiatest::Reporter* reporter) {
#endif
}
static void test_free_resource_messages(skiatest::Reporter* reporter) {
Mock mock(10, 30000);
GrContext* context = mock.context();
GrResourceCache* cache = mock.cache();
GrGpu* gpu = context->contextPriv().getGpu();
TestResource* wrapped1 = TestResource::CreateWrapped(gpu, 12);
cache->insertCrossContextGpuResource(wrapped1);
REPORTER_ASSERT(reporter, 1 == TestResource::NumAlive());
TestResource* wrapped2 = TestResource::CreateWrapped(gpu, 12);
cache->insertCrossContextGpuResource(wrapped2);
REPORTER_ASSERT(reporter, 2 == TestResource::NumAlive());
// Have only ref waiting on message.
wrapped1->unref();
wrapped2->unref();
REPORTER_ASSERT(reporter, 2 == TestResource::NumAlive());
// This should free nothing since no messages were sent.
cache->purgeAsNeeded();
// Send message to free the first resource
GrGpuResourceFreedMessage msg { wrapped1, context->uniqueID() };
SkMessageBus<GrGpuResourceFreedMessage>::Post(msg);
cache->purgeAsNeeded();
REPORTER_ASSERT(reporter, 1 == TestResource::NumAlive());
mock.reset();
REPORTER_ASSERT(reporter, 0 == TestResource::NumAlive());
}
DEF_GPUTEST(ResourceCacheMisc, reporter, /* options */) {
// The below tests create their own mock contexts.
test_no_key(reporter);
@ -1616,6 +1658,7 @@ DEF_GPUTEST(ResourceCacheMisc, reporter, /* options */) {
test_custom_data(reporter);
test_abandoned(reporter);
test_tags(reporter);
test_free_resource_messages(reporter);
}
////////////////////////////////////////////////////////////////////////////////