diff --git a/src/backend/PipelineLayout.cpp b/src/backend/PipelineLayout.cpp index 6adca64f29..1b0e528b8a 100644 --- a/src/backend/PipelineLayout.cpp +++ b/src/backend/PipelineLayout.cpp @@ -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(); } } diff --git a/src/backend/RefCounted.cpp b/src/backend/RefCounted.cpp index c5a56c1f6c..133dd5c354 100644 --- a/src/backend/RefCounted.cpp +++ b/src/backend/RefCounted.cpp @@ -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(); } diff --git a/src/tests/unittests/RefCountedTests.cpp b/src/tests/unittests/RefCountedTests.cpp index 1ba3a97628..f5b0007f22 100644 --- a/src/tests/unittests/RefCountedTests.cpp +++ b/src/tests/unittests/RefCountedTests.cpp @@ -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;