From 7e8385c1830964bbe5ff7bd3bb51875ababffb8c Mon Sep 17 00:00:00 2001 From: Rafael Cintron Date: Mon, 20 Apr 2020 17:36:22 +0000 Subject: [PATCH] Move and improve RefCounted - Move RefCounted to common (from dawn_native) so that we can use it from additional places. - Use EXPECT_ macros instead of ASSERT_ in RefCounted tests for improved logging on failures. - Add a missing test for Ref::Detach. - Plug memory leak in RaceOnReferenceRelease Change-Id: Iaa7b11b5a6fa146e3c322143279a21a4ac027547 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/19903 Reviewed-by: Corentin Wallez Reviewed-by: Austin Eng Commit-Queue: Rafael Cintron --- src/common/BUILD.gn | 2 + src/common/CMakeLists.txt | 2 + src/common/RefCounted.cpp | 78 +++++++++++ src/common/RefCounted.h | 139 +++++++++++++++++++ src/dawn_native/BUILD.gn | 2 - src/dawn_native/CMakeLists.txt | 2 - src/dawn_native/ErrorScope.h | 2 +- src/dawn_native/ErrorScopeTracker.h | 2 +- src/dawn_native/FenceSignalTracker.h | 2 +- src/dawn_native/Forward.h | 6 +- src/dawn_native/Instance.h | 2 +- src/dawn_native/ObjectBase.h | 2 +- src/dawn_native/RefCounted.cpp | 83 ------------ src/dawn_native/RefCounted.h | 143 -------------------- src/dawn_native/Surface.h | 2 +- src/tests/unittests/ExtensionTests.cpp | 2 +- src/tests/unittests/GetProcAddressTests.cpp | 2 +- src/tests/unittests/RefCountedTests.cpp | 94 +++++++------ src/tests/unittests/ToBackendTests.cpp | 2 +- 19 files changed, 289 insertions(+), 280 deletions(-) create mode 100644 src/common/RefCounted.cpp create mode 100644 src/common/RefCounted.h delete mode 100644 src/dawn_native/RefCounted.cpp delete mode 100644 src/dawn_native/RefCounted.h diff --git a/src/common/BUILD.gn b/src/common/BUILD.gn index dfa5109220..61cadaa86f 100644 --- a/src/common/BUILD.gn +++ b/src/common/BUILD.gn @@ -131,6 +131,8 @@ if (is_win || is_linux || is_mac || is_fuchsia || is_android) { "Math.h", "PlacementAllocated.h", "Platform.h", + "RefCounted.cpp", + "RefCounted.h", "Result.cpp", "Result.h", "Serial.h", diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index be9bd84b96..2e909b33b4 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -31,6 +31,8 @@ target_sources(dawn_common PRIVATE "Math.h" "PlacementAllocated.h" "Platform.h" + "RefCounted.cpp" + "RefCounted.h" "Result.cpp" "Result.h" "Serial.h" diff --git a/src/common/RefCounted.cpp b/src/common/RefCounted.cpp new file mode 100644 index 0000000000..bb0f76d884 --- /dev/null +++ b/src/common/RefCounted.cpp @@ -0,0 +1,78 @@ +// Copyright 2017 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "common/RefCounted.h" + +#include "common/Assert.h" + +#include + +static constexpr size_t kPayloadBits = 1; +static constexpr uint64_t kPayloadMask = (uint64_t(1) << kPayloadBits) - 1; +static constexpr uint64_t kRefCountIncrement = (uint64_t(1) << kPayloadBits); + +RefCounted::RefCounted(uint64_t payload) : mRefCount(kRefCountIncrement + payload) { + ASSERT((payload & kPayloadMask) == payload); +} + +uint64_t RefCounted::GetRefCountForTesting() const { + return mRefCount >> kPayloadBits; +} + +uint64_t RefCounted::GetRefCountPayload() const { + // We only care about the payload bits of the refcount. These never change after + // initialization so we can use the relaxed memory order. The order doesn't guarantee + // anything except the atomicity of the load, which is enough since any past values of the + // atomic will have the correct payload bits. + return kPayloadMask & mRefCount.load(std::memory_order_relaxed); +} + +void RefCounted::Reference() { + ASSERT((mRefCount & ~kPayloadMask) != 0); + + // The relaxed ordering guarantees only the atomicity of the update, which is enough here + // because the reference we are copying from still exists and makes sure other threads + // don't delete `this`. + // See the explanation in the Boost documentation: + // https://www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html + mRefCount.fetch_add(kRefCountIncrement, std::memory_order_relaxed); +} + +void RefCounted::Release() { + ASSERT((mRefCount & ~kPayloadMask) != 0); + + // The release fence here is to make sure all accesses to the object on a thread A + // happen-before the object is deleted on a thread B. The release memory order ensures that + // all accesses on thread A happen-before the refcount is decreased and the atomic variable + // makes sure the refcount decrease in A happens-before the refcount decrease in B. Finally + // the acquire fence in the destruction case makes sure the refcount decrease in B + // happens-before the `delete this`. + // + // See the explanation in the Boost documentation: + // https://www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html + uint64_t previousRefCount = mRefCount.fetch_sub(kRefCountIncrement, std::memory_order_release); + + // Check that the previous reference count was strictly less than 2, ignoring payload bits. + if (previousRefCount < 2 * kRefCountIncrement) { + // Note that on ARM64 this will generate a `dmb ish` instruction which is a global + // memory barrier, when an acquire load on mRefCount (using the `ldar` instruction) + // should be enough and could end up being faster. + std::atomic_thread_fence(std::memory_order_acquire); + DeleteThis(); + } +} + +void RefCounted::DeleteThis() { + delete this; +} \ No newline at end of file diff --git a/src/common/RefCounted.h b/src/common/RefCounted.h new file mode 100644 index 0000000000..05cef94784 --- /dev/null +++ b/src/common/RefCounted.h @@ -0,0 +1,139 @@ +// Copyright 2017 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef COMMON_REFCOUNTED_H_ +#define COMMON_REFCOUNTED_H_ + +#include +#include + +class RefCounted { + public: + RefCounted(uint64_t payload = 0); + + uint64_t GetRefCountForTesting() const; + uint64_t GetRefCountPayload() const; + + // Dawn API + void Reference(); + void Release(); + + protected: + virtual ~RefCounted() = default; + // A Derived class may override this if they require a custom deleter. + virtual void DeleteThis(); + + private: + std::atomic_uint64_t mRefCount; +}; + +template +class Ref { + public: + Ref() { + } + + Ref(T* p) : mPointee(p) { + Reference(); + } + + Ref(const Ref& other) : mPointee(other.mPointee) { + Reference(); + } + Ref& operator=(const Ref& other) { + if (&other == this) + return *this; + + other.Reference(); + Release(); + mPointee = other.mPointee; + + return *this; + } + + Ref(Ref&& other) { + mPointee = other.mPointee; + other.mPointee = nullptr; + } + Ref& operator=(Ref&& other) { + if (&other == this) + return *this; + + Release(); + mPointee = other.mPointee; + other.mPointee = nullptr; + + return *this; + } + + ~Ref() { + Release(); + mPointee = nullptr; + } + + operator bool() { + return mPointee != nullptr; + } + + const T& operator*() const { + return *mPointee; + } + T& operator*() { + return *mPointee; + } + + const T* operator->() const { + return mPointee; + } + T* operator->() { + return mPointee; + } + + const T* Get() const { + return mPointee; + } + T* Get() { + return mPointee; + } + + T* Detach() { + T* pointee = mPointee; + mPointee = nullptr; + return pointee; + } + + private: + void Reference() const { + if (mPointee != nullptr) { + mPointee->Reference(); + } + } + void Release() const { + if (mPointee != nullptr) { + mPointee->Release(); + } + } + + // static_assert(std::is_base_of::value, ""); + T* mPointee = nullptr; +}; + +template +Ref AcquireRef(T* pointee) { + Ref ref(pointee); + ref->Release(); + return ref; +} + +#endif // COMMON_REFCOUNTED_H_ diff --git a/src/dawn_native/BUILD.gn b/src/dawn_native/BUILD.gn index 37db8568dc..908128c4e4 100644 --- a/src/dawn_native/BUILD.gn +++ b/src/dawn_native/BUILD.gn @@ -230,8 +230,6 @@ source_set("dawn_native_sources") { "ProgrammablePassEncoder.h", "Queue.cpp", "Queue.h", - "RefCounted.cpp", - "RefCounted.h", "RenderBundle.cpp", "RenderBundle.h", "RenderBundleEncoder.cpp", diff --git a/src/dawn_native/CMakeLists.txt b/src/dawn_native/CMakeLists.txt index 72053f38e0..638b41f717 100644 --- a/src/dawn_native/CMakeLists.txt +++ b/src/dawn_native/CMakeLists.txt @@ -102,8 +102,6 @@ target_sources(dawn_native PRIVATE "ProgrammablePassEncoder.h" "Queue.cpp" "Queue.h" - "RefCounted.cpp" - "RefCounted.h" "RenderBundle.cpp" "RenderBundle.h" "RenderBundleEncoder.cpp" diff --git a/src/dawn_native/ErrorScope.h b/src/dawn_native/ErrorScope.h index f3a0187f8d..eea066def4 100644 --- a/src/dawn_native/ErrorScope.h +++ b/src/dawn_native/ErrorScope.h @@ -17,7 +17,7 @@ #include "dawn_native/dawn_platform.h" -#include "dawn_native/RefCounted.h" +#include "common/RefCounted.h" #include diff --git a/src/dawn_native/ErrorScopeTracker.h b/src/dawn_native/ErrorScopeTracker.h index 7337eb6414..2ca651198f 100644 --- a/src/dawn_native/ErrorScopeTracker.h +++ b/src/dawn_native/ErrorScopeTracker.h @@ -15,8 +15,8 @@ #ifndef DAWNNATIVE_ERRORSCOPETRACKER_H_ #define DAWNNATIVE_ERRORSCOPETRACKER_H_ +#include "common/RefCounted.h" #include "common/SerialQueue.h" -#include "dawn_native/RefCounted.h" namespace dawn_native { diff --git a/src/dawn_native/FenceSignalTracker.h b/src/dawn_native/FenceSignalTracker.h index 53333e946b..10026a44ef 100644 --- a/src/dawn_native/FenceSignalTracker.h +++ b/src/dawn_native/FenceSignalTracker.h @@ -15,8 +15,8 @@ #ifndef DAWNNATIVE_FENCESIGNALTRACKER_H_ #define DAWNNATIVE_FENCESIGNALTRACKER_H_ +#include "common/RefCounted.h" #include "common/SerialQueue.h" -#include "dawn_native/RefCounted.h" namespace dawn_native { diff --git a/src/dawn_native/Forward.h b/src/dawn_native/Forward.h index 05538e85ee..be7c98f9e3 100644 --- a/src/dawn_native/Forward.h +++ b/src/dawn_native/Forward.h @@ -17,6 +17,9 @@ #include +template +class Ref; + namespace dawn_native { class AdapterBase; @@ -48,9 +51,6 @@ namespace dawn_native { class DeviceBase; - template - class Ref; - template class PerStage; diff --git a/src/dawn_native/Instance.h b/src/dawn_native/Instance.h index e07d04b6b4..f297c03bd4 100644 --- a/src/dawn_native/Instance.h +++ b/src/dawn_native/Instance.h @@ -15,10 +15,10 @@ #ifndef DAWNNATIVE_INSTANCE_H_ #define DAWNNATIVE_INSTANCE_H_ +#include "common/RefCounted.h" #include "dawn_native/Adapter.h" #include "dawn_native/BackendConnection.h" #include "dawn_native/Extensions.h" -#include "dawn_native/RefCounted.h" #include "dawn_native/Toggles.h" #include "dawn_native/dawn_platform.h" diff --git a/src/dawn_native/ObjectBase.h b/src/dawn_native/ObjectBase.h index cb627a02a5..544ce1a4bb 100644 --- a/src/dawn_native/ObjectBase.h +++ b/src/dawn_native/ObjectBase.h @@ -15,7 +15,7 @@ #ifndef DAWNNATIVE_OBJECTBASE_H_ #define DAWNNATIVE_OBJECTBASE_H_ -#include "dawn_native/RefCounted.h" +#include "common/RefCounted.h" namespace dawn_native { diff --git a/src/dawn_native/RefCounted.cpp b/src/dawn_native/RefCounted.cpp deleted file mode 100644 index ceb9c75fc6..0000000000 --- a/src/dawn_native/RefCounted.cpp +++ /dev/null @@ -1,83 +0,0 @@ -// Copyright 2017 The Dawn Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "dawn_native/RefCounted.h" - -#include "common/Assert.h" - -#include - -namespace dawn_native { - - static constexpr size_t kPayloadBits = 1; - static constexpr uint64_t kPayloadMask = (uint64_t(1) << kPayloadBits) - 1; - static constexpr uint64_t kRefCountIncrement = (uint64_t(1) << kPayloadBits); - - RefCounted::RefCounted(uint64_t payload) : mRefCount(kRefCountIncrement + payload) { - ASSERT((payload & kPayloadMask) == payload); - } - - uint64_t RefCounted::GetRefCountForTesting() const { - return mRefCount >> kPayloadBits; - } - - uint64_t RefCounted::GetRefCountPayload() const { - // We only care about the payload bits of the refcount. These never change after - // initialization so we can use the relaxed memory order. The order doesn't guarantee - // anything except the atomicity of the load, which is enough since any past values of the - // atomic will have the correct payload bits. - return kPayloadMask & mRefCount.load(std::memory_order_relaxed); - } - - void RefCounted::Reference() { - ASSERT((mRefCount & ~kPayloadMask) != 0); - - // The relaxed ordering guarantees only the atomicity of the update, which is enough here - // because the reference we are copying from still exists and makes sure other threads - // don't delete `this`. - // See the explanation in the Boost documentation: - // https://www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html - mRefCount.fetch_add(kRefCountIncrement, std::memory_order_relaxed); - } - - void RefCounted::Release() { - ASSERT((mRefCount & ~kPayloadMask) != 0); - - // The release fence here is to make sure all accesses to the object on a thread A - // happen-before the object is deleted on a thread B. The release memory order ensures that - // all accesses on thread A happen-before the refcount is decreased and the atomic variable - // makes sure the refcount decrease in A happens-before the refcount decrease in B. Finally - // the acquire fence in the destruction case makes sure the refcount decrease in B - // happens-before the `delete this`. - // - // See the explanation in the Boost documentation: - // https://www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html - uint64_t previousRefCount = - mRefCount.fetch_sub(kRefCountIncrement, std::memory_order_release); - - // Check that the previous reference count was strictly less than 2, ignoring payload bits. - if (previousRefCount < 2 * kRefCountIncrement) { - // Note that on ARM64 this will generate a `dmb ish` instruction which is a global - // memory barrier, when an acquire load on mRefCount (using the `ldar` instruction) - // should be enough and could end up being faster. - std::atomic_thread_fence(std::memory_order_acquire); - DeleteThis(); - } - } - - void RefCounted::DeleteThis() { - delete this; - } - -} // namespace dawn_native diff --git a/src/dawn_native/RefCounted.h b/src/dawn_native/RefCounted.h deleted file mode 100644 index 3b3a55e3bb..0000000000 --- a/src/dawn_native/RefCounted.h +++ /dev/null @@ -1,143 +0,0 @@ -// Copyright 2017 The Dawn Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef DAWNNATIVE_REFCOUNTED_H_ -#define DAWNNATIVE_REFCOUNTED_H_ - -#include -#include - -namespace dawn_native { - - class RefCounted { - public: - RefCounted(uint64_t payload = 0); - - uint64_t GetRefCountForTesting() const; - uint64_t GetRefCountPayload() const; - - // Dawn API - void Reference(); - void Release(); - - protected: - virtual ~RefCounted() = default; - // A Derived class may override this if they require a custom deleter. - virtual void DeleteThis(); - - private: - std::atomic_uint64_t mRefCount; - }; - - template - class Ref { - public: - Ref() { - } - - Ref(T* p) : mPointee(p) { - Reference(); - } - - Ref(const Ref& other) : mPointee(other.mPointee) { - Reference(); - } - Ref& operator=(const Ref& other) { - if (&other == this) - return *this; - - other.Reference(); - Release(); - mPointee = other.mPointee; - - return *this; - } - - Ref(Ref&& other) { - mPointee = other.mPointee; - other.mPointee = nullptr; - } - Ref& operator=(Ref&& other) { - if (&other == this) - return *this; - - Release(); - mPointee = other.mPointee; - other.mPointee = nullptr; - - return *this; - } - - ~Ref() { - Release(); - mPointee = nullptr; - } - - operator bool() { - return mPointee != nullptr; - } - - const T& operator*() const { - return *mPointee; - } - T& operator*() { - return *mPointee; - } - - const T* operator->() const { - return mPointee; - } - T* operator->() { - return mPointee; - } - - const T* Get() const { - return mPointee; - } - T* Get() { - return mPointee; - } - - T* Detach() { - T* pointee = mPointee; - mPointee = nullptr; - return pointee; - } - - private: - void Reference() const { - if (mPointee != nullptr) { - mPointee->Reference(); - } - } - void Release() const { - if (mPointee != nullptr) { - mPointee->Release(); - } - } - - // static_assert(std::is_base_of::value, ""); - T* mPointee = nullptr; - }; - - template - Ref AcquireRef(T* pointee) { - Ref ref(pointee); - ref->Release(); - return ref; - } - -} // namespace dawn_native - -#endif // DAWNNATIVE_REFCOUNTED_H_ diff --git a/src/dawn_native/Surface.h b/src/dawn_native/Surface.h index 96748b7925..048298b00b 100644 --- a/src/dawn_native/Surface.h +++ b/src/dawn_native/Surface.h @@ -15,9 +15,9 @@ #ifndef DAWNNATIVE_SURFACE_H_ #define DAWNNATIVE_SURFACE_H_ +#include "common/RefCounted.h" #include "dawn_native/Error.h" #include "dawn_native/Forward.h" -#include "dawn_native/RefCounted.h" #include "dawn_native/dawn_platform.h" diff --git a/src/tests/unittests/ExtensionTests.cpp b/src/tests/unittests/ExtensionTests.cpp index 2805e11fc4..4e74f8e946 100644 --- a/src/tests/unittests/ExtensionTests.cpp +++ b/src/tests/unittests/ExtensionTests.cpp @@ -38,7 +38,7 @@ class ExtensionTests : public testing::Test { static_cast(dawn_native::Extension::EnumCount); protected: - dawn_native::Ref mInstanceBase; + Ref mInstanceBase; dawn_native::null::Adapter mAdapterBase; }; diff --git a/src/tests/unittests/GetProcAddressTests.cpp b/src/tests/unittests/GetProcAddressTests.cpp index fe454465da..f5ac8c699a 100644 --- a/src/tests/unittests/GetProcAddressTests.cpp +++ b/src/tests/unittests/GetProcAddressTests.cpp @@ -90,7 +90,7 @@ namespace { } protected: - dawn_native::Ref mNativeInstance; + Ref mNativeInstance; dawn_native::null::Adapter mNativeAdapter; std::unique_ptr mC2sBuf; diff --git a/src/tests/unittests/RefCountedTests.cpp b/src/tests/unittests/RefCountedTests.cpp index cbba420fe4..a7ddacff72 100644 --- a/src/tests/unittests/RefCountedTests.cpp +++ b/src/tests/unittests/RefCountedTests.cpp @@ -15,9 +15,7 @@ #include #include -#include "dawn_native/RefCounted.h" - -using namespace dawn_native; +#include "common/RefCounted.h" struct RCTest : public RefCounted { RCTest() : RefCounted() { @@ -48,7 +46,7 @@ TEST(RefCounted, StartsWithOneRef) { auto test = new RCTest(&deleted); test->Release(); - ASSERT_TRUE(deleted); + EXPECT_TRUE(deleted); } // Test adding refs keep the RC alive. @@ -58,10 +56,10 @@ TEST(RefCounted, AddingRefKeepsAlive) { test->Reference(); test->Release(); - ASSERT_FALSE(deleted); + EXPECT_FALSE(deleted); test->Release(); - ASSERT_TRUE(deleted); + EXPECT_TRUE(deleted); } // Test that Reference and Release atomically change the refcount. @@ -79,7 +77,7 @@ TEST(RefCounted, RaceOnReferenceRelease) { t1.join(); t2.join(); - ASSERT_EQ(test->GetRefCountForTesting(), 200001u); + EXPECT_EQ(test->GetRefCountForTesting(), 200001u); auto releaseManyTimes = [test]() { for (uint32_t i = 0; i < 100000; ++i) { @@ -91,7 +89,10 @@ TEST(RefCounted, RaceOnReferenceRelease) { std::thread t4(releaseManyTimes); t3.join(); t4.join(); - ASSERT_EQ(test->GetRefCountForTesting(), 1u); + EXPECT_EQ(test->GetRefCountForTesting(), 1u); + + test->Release(); + EXPECT_TRUE(deleted); } // Test Ref remove reference when going out of scope @@ -101,7 +102,7 @@ TEST(Ref, EndOfScopeRemovesRef) { Ref test(new RCTest(&deleted)); test->Release(); } - ASSERT_TRUE(deleted); + EXPECT_TRUE(deleted); } // Test getting pointer out of the Ref @@ -110,18 +111,18 @@ TEST(Ref, Gets) { Ref test(original); test->Release(); - ASSERT_EQ(test.Get(), original); - ASSERT_EQ(&*test, original); - ASSERT_EQ(test->GetThis(), original); + EXPECT_EQ(test.Get(), original); + EXPECT_EQ(&*test, original); + EXPECT_EQ(test->GetThis(), original); } // Test Refs default to null TEST(Ref, DefaultsToNull) { Ref test; - ASSERT_EQ(test.Get(), nullptr); - ASSERT_EQ(&*test, nullptr); - ASSERT_EQ(test->GetThis(), nullptr); + EXPECT_EQ(test.Get(), nullptr); + EXPECT_EQ(&*test, nullptr); + EXPECT_EQ(test->GetThis(), nullptr); } // Test Refs can be used inside ifs @@ -131,7 +132,7 @@ TEST(Ref, BoolConversion) { full->Release(); if (!full || empty) { - ASSERT_TRUE(false); + EXPECT_TRUE(false); } } @@ -144,13 +145,13 @@ TEST(Ref, CopyConstructor) { Ref destination(source); original->Release(); - ASSERT_EQ(source.Get(), original); - ASSERT_EQ(destination.Get(), original); + EXPECT_EQ(source.Get(), original); + EXPECT_EQ(destination.Get(), original); source = nullptr; - ASSERT_FALSE(deleted); + EXPECT_FALSE(deleted); destination = nullptr; - ASSERT_TRUE(deleted); + EXPECT_TRUE(deleted); } // Test Ref's copy assignment @@ -164,15 +165,15 @@ TEST(Ref, CopyAssignment) { Ref destination; destination = source; - ASSERT_EQ(source.Get(), original); - ASSERT_EQ(destination.Get(), original); + EXPECT_EQ(source.Get(), original); + EXPECT_EQ(destination.Get(), original); source = nullptr; // This fails when address sanitizer is turned on - ASSERT_FALSE(deleted); + EXPECT_FALSE(deleted); destination = nullptr; - ASSERT_TRUE(deleted); + EXPECT_TRUE(deleted); } // Test Ref's move constructor @@ -184,12 +185,12 @@ TEST(Ref, MoveConstructor) { Ref destination(std::move(source)); original->Release(); - ASSERT_EQ(source.Get(), nullptr); - ASSERT_EQ(destination.Get(), original); - ASSERT_FALSE(deleted); + EXPECT_EQ(source.Get(), nullptr); + EXPECT_EQ(destination.Get(), original); + EXPECT_FALSE(deleted); destination = nullptr; - ASSERT_TRUE(deleted); + EXPECT_TRUE(deleted); } // Test Ref's move assignment @@ -203,38 +204,55 @@ TEST(Ref, MoveAssignment) { Ref destination; destination = std::move(source); - ASSERT_EQ(source.Get(), nullptr); - ASSERT_EQ(destination.Get(), original); - ASSERT_FALSE(deleted); + EXPECT_EQ(source.Get(), nullptr); + EXPECT_EQ(destination.Get(), original); + EXPECT_FALSE(deleted); destination = nullptr; - ASSERT_TRUE(deleted); + EXPECT_TRUE(deleted); } // Test the payload initial value is set correctly TEST(Ref, InitialPayloadValue) { RCTest* testDefaultConstructor = new RCTest(); - ASSERT_EQ(testDefaultConstructor->GetRefCountPayload(), 0u); + EXPECT_EQ(testDefaultConstructor->GetRefCountPayload(), 0u); testDefaultConstructor->Release(); RCTest* testZero = new RCTest(uint64_t(0ull)); - ASSERT_EQ(testZero->GetRefCountPayload(), 0u); + EXPECT_EQ(testZero->GetRefCountPayload(), 0u); testZero->Release(); RCTest* testOne = new RCTest(1ull); - ASSERT_EQ(testOne->GetRefCountPayload(), 1u); + EXPECT_EQ(testOne->GetRefCountPayload(), 1u); testOne->Release(); } // Test that the payload survives ref and release operations TEST(Ref, PayloadUnchangedByRefCounting) { RCTest* test = new RCTest(1ull); - ASSERT_EQ(test->GetRefCountPayload(), 1u); + EXPECT_EQ(test->GetRefCountPayload(), 1u); test->Reference(); - ASSERT_EQ(test->GetRefCountPayload(), 1u); + EXPECT_EQ(test->GetRefCountPayload(), 1u); test->Release(); - ASSERT_EQ(test->GetRefCountPayload(), 1u); + EXPECT_EQ(test->GetRefCountPayload(), 1u); test->Release(); } + +// Test that Detach pulls out the pointer and stops tracking it. +TEST(Ref, Detach) { + bool deleted = false; + RCTest* original = new RCTest(&deleted); + + Ref test(original); + original->Release(); + + RCTest* detached = test.Detach(); + EXPECT_EQ(detached, original); + EXPECT_EQ(detached->GetRefCountForTesting(), 1u); + EXPECT_EQ(test.Get(), nullptr); + + detached->Release(); + EXPECT_TRUE(deleted); +} \ No newline at end of file diff --git a/src/tests/unittests/ToBackendTests.cpp b/src/tests/unittests/ToBackendTests.cpp index d16a4f7096..c537a8e959 100644 --- a/src/tests/unittests/ToBackendTests.cpp +++ b/src/tests/unittests/ToBackendTests.cpp @@ -14,7 +14,7 @@ #include -#include "dawn_native/RefCounted.h" +#include "common/RefCounted.h" #include "dawn_native/ToBackend.h" #include