Fix BindGroupLayout leak in PipelineLayout default.

PipelineLayout was re-entering in NXT to create the default empty BGLs,
but forgot to remove the initial external refcount for them.

Fixing this showed an issue where it was impossible to externally
reference a RefCounted that had only internal references, fix this as
well and add a test.
This commit is contained in:
Corentin Wallez 2018-04-17 16:37:07 -04:00 committed by Corentin Wallez
parent df5a18d883
commit bfeb285dcf
3 changed files with 33 additions and 1 deletions

View File

@ -60,6 +60,8 @@ namespace backend {
for (size_t group = 0; group < kMaxBindGroups; ++group) {
if (!mBindGroupLayouts[group]) {
mBindGroupLayouts[group] = mDevice->CreateBindGroupLayoutBuilder()->GetResult();
// Remove the external ref objects are created with
mBindGroupLayouts[group]->Release();
}
}

View File

@ -26,13 +26,16 @@ namespace backend {
void RefCounted::ReferenceInternal() {
ASSERT(mInternalRefs != 0);
// TODO(cwallez@chromium.org): what to do on overflow?
mInternalRefs++;
}
void RefCounted::ReleaseInternal() {
ASSERT(mInternalRefs != 0);
mInternalRefs--;
if (mInternalRefs == 0) {
ASSERT(mExternalRefs == 0);
// TODO(cwallez@chromium.org): would this work with custom allocators?
@ -49,14 +52,23 @@ namespace backend {
}
void RefCounted::Reference() {
ASSERT(mExternalRefs != 0);
ASSERT(mInternalRefs != 0);
// mExternalRefs != 0 counts as one internal ref.
if (mExternalRefs == 0) {
ReferenceInternal();
}
// TODO(cwallez@chromium.org): what to do on overflow?
mExternalRefs++;
}
void RefCounted::Release() {
ASSERT(mInternalRefs != 0);
ASSERT(mExternalRefs != 0);
mExternalRefs--;
// mExternalRefs != 0 counts as one internal ref.
if (mExternalRefs == 0) {
ReleaseInternal();
}

View File

@ -60,6 +60,24 @@ TEST(RefCounted, InternalRefKeepsAlive) {
ASSERT_TRUE(deleted);
}
// Test that when adding an external ref from 0, an internal ref is added
TEST(RefCounted, AddExternalRefFromZero) {
bool deleted = false;
auto test = new RCTest(&deleted);
test->ReferenceInternal();
test->Release();
ASSERT_FALSE(deleted);
// Reference adds an internal ref and release removes one
test->Reference();
test->Release();
ASSERT_FALSE(deleted);
test->ReleaseInternal();
ASSERT_TRUE(deleted);
}
// Test Ref remove internal reference when going out of scope
TEST(Ref, EndOfScopeRemovesInternalRef) {
bool deleted = false;