diff --git a/Substrate/Include/Substrate/AllocatorBase.h b/Substrate/Include/Substrate/AllocatorBase.h index c1234ac..ef15f08 100644 --- a/Substrate/Include/Substrate/AllocatorBase.h +++ b/Substrate/Include/Substrate/AllocatorBase.h @@ -14,6 +14,7 @@ namespace Substrate { virtual size_t GetUsedMemory() const = 0; virtual size_t GetCurrentAllocationCount() const = 0; virtual size_t GetTotalAllocationCount() const = 0; + virtual size_t GetMaxedGenerationCount() const = 0; virtual size_t GetResetCount() const = 0; #endif }; diff --git a/Substrate/Include/Substrate/BaseHandle.h b/Substrate/Include/Substrate/BaseHandle.h index fc774fd..1f7f96a 100644 --- a/Substrate/Include/Substrate/BaseHandle.h +++ b/Substrate/Include/Substrate/BaseHandle.h @@ -52,6 +52,8 @@ namespace Substrate { class BaseHandle { public: + static_assert(GenerationMask != static_cast(~static_cast(0)), "GenerationMask cannot be all 1s"); + constexpr BaseHandle() : m_Handle(0) {}; constexpr BaseHandle(THandleType index) : m_Handle(index) { @@ -76,23 +78,14 @@ namespace Substrate { } - /// Only true if the generation is not equal to the generation mask. Except when there are no generation bits, which is always a valid generation. Returns false if index == indexMask + /// Only true if the generation is not equal to the generation mask. This also covers GenerationMask = 0 -> the only invalid handle IS INVALID_HANDLE -> Index maxed out. + /// Or, generationBits maxed and index maxed, which is invalid because generation is maxed out. constexpr bool IsValid() { - //If the handle's index bits are all set to 1, the handle is invalid. - constexpr THandleType indexMask = GetIndexMask(); - if ((m_Handle & indexMask) == indexMask) - return false; - - //If the generation mask is 0, the handle is always valid. - if(GenerationMask == 0) + if constexpr(GenerationMask == 0) return true; - - //We return false if the generation mask is all bits set, because that means there is no valid index possible. - if (GenerationMask == static_cast((~static_cast(0)))) - return false; - - return (m_Handle & GenerationMask) != GenerationMask; + else + return (m_Handle & GenerationMask) != GenerationMask; } /// This is only true if the generations are equal AND the index! diff --git a/Substrate/Include/Substrate/LinearAllocator.h b/Substrate/Include/Substrate/LinearAllocator.h index 4b16241..401c67b 100644 --- a/Substrate/Include/Substrate/LinearAllocator.h +++ b/Substrate/Include/Substrate/LinearAllocator.h @@ -35,6 +35,7 @@ namespace Substrate { inline size_t GetTotalMemory() const override { return m_TotalSize; } inline size_t GetTotalAllocationCount() const override { return m_TotalAllocationCount; } inline size_t GetCurrentAllocationCount() const override { return m_CurrentAllocationCount; } + inline size_t GetMaxedGenerationCount() const override { return 0; } inline size_t GetResetCount() const override { return m_ResetCount; } #endif private: diff --git a/Substrate/Include/Substrate/PoolAllocator.h b/Substrate/Include/Substrate/PoolAllocator.h index cf5d374..02d786c 100644 --- a/Substrate/Include/Substrate/PoolAllocator.h +++ b/Substrate/Include/Substrate/PoolAllocator.h @@ -38,7 +38,7 @@ namespace Substrate { bool IsHandleValid(TResourceHandle handle); void Free(TResourceHandle handle); - const std::vector& GetFreeHandleIndices() const { return m_FreeHandles; } + const uint32_t GetFreeHandleCount() const { return m_FreeCount; } using InternalHandle = DefineHandle; const std::vector& GetHandles() const { return m_Handles; } @@ -48,6 +48,7 @@ namespace Substrate { inline size_t GetUsedMemory() const override { return m_CurrentAllocationCount * sizeof(TBlockType); } inline size_t GetCurrentAllocationCount() const override { return m_CurrentAllocationCount; } inline size_t GetTotalAllocationCount() const override { return m_TotalAllocationCount; } + inline size_t GetMaxedGenerationCount() const override { return m_MaxedGenerationCount; } inline size_t GetResetCount() const override { return 0; } #endif private: @@ -59,6 +60,10 @@ namespace Substrate { size_t m_TotalAllocationCount = 0; std::vector m_Handles; std::vector m_FreeHandles; + uint32_t m_FreeHead = 0; + uint32_t m_FreeTail = 0; + uint32_t m_FreeCount = 0; + uint32_t m_MaxedGenerationCount = 0; }; @@ -75,11 +80,15 @@ namespace Substrate { m_Handles.reserve(MAX_BLOCK_COUNT); m_FreeHandles.reserve(MAX_BLOCK_COUNT); + m_FreeCount = MAX_BLOCK_COUNT; + m_FreeHead = 0; + m_FreeTail = 0; + // Initialize handles in reverse order for better cache locality for (uint32_t i = 0; i < MAX_BLOCK_COUNT; i++) { m_Handles.push_back(InternalHandle(i)); - m_FreeHandles.push_back(MAX_BLOCK_COUNT-1-i); + m_FreeHandles.push_back(i); } } @@ -108,6 +117,10 @@ namespace Substrate { m_TotalSize = 0; m_Handles.clear(); m_FreeHandles.clear(); + m_FreeHead = 0; + m_FreeTail = 0; + //Currently blocks further usage of the Allocator + m_FreeCount = 0; } template @@ -115,17 +128,18 @@ namespace Substrate { TResourceHandle PoolAllocator::Allocate() { // Check if there are free handles - if(m_FreeHandles.size() == 0) + if(m_FreeCount == 0) throw AllocatorOutOfMemoryException("Pool allocator out of memory"); // Get the next free handle - uint32_t idx = m_FreeHandles.back(); - m_FreeHandles.pop_back(); + uint32_t idx = m_FreeHandles[m_FreeHead]; + m_FreeHead = (m_FreeHead + 1) % MAX_BLOCK_COUNT; //Calls constructor -> sets default values new(m_MemoryBlock + idx) TBlockType{}; m_CurrentAllocationCount++; m_TotalAllocationCount++; + m_FreeCount--; return m_Handles[idx].GetRaw(); } @@ -135,9 +149,9 @@ namespace Substrate { { InternalHandle handle = InternalHandle::FromRawType(resourceHandle); InternalHandle& internal = m_Handles[handle.Index()]; - if (!internal.Equals(handle)) - return false; - return true; + if (internal.IsValid() && internal.Equals(handle)) + return true; + return false; } template @@ -161,16 +175,21 @@ namespace Substrate { // Validate handle InternalHandle handle = InternalHandle::FromRawType(resourceHandle); InternalHandle& internal = m_Handles[handle.Index()]; - if (!internal.Equals(handle)) + if (!internal.Equals(handle) || !internal.IsValid()) return; // Invalid handle or handle was already freed internal = internal.IncrementGeneration(); // Check if generation is maxed out - if(!internal.IsValid()) + if (!internal.IsValid()) + { + m_MaxedGenerationCount++; return; // Cannot free handle anymore + } - m_FreeHandles.push_back(internal.Index()); + m_FreeHandles[m_FreeTail] = internal.Index(); + m_FreeCount++; + m_FreeTail = (m_FreeTail + 1) % MAX_BLOCK_COUNT; // Decrease allocation count only if handle was put back into the free list m_CurrentAllocationCount--; diff --git a/Substrate/Include/Substrate/StackAllocator.h b/Substrate/Include/Substrate/StackAllocator.h index 79b0265..5d9b6be 100644 --- a/Substrate/Include/Substrate/StackAllocator.h +++ b/Substrate/Include/Substrate/StackAllocator.h @@ -25,6 +25,7 @@ namespace Substrate { inline size_t GetTotalMemory() const override { return m_TotalSize; } inline size_t GetTotalAllocationCount() const override { return m_TotalAllocationCount; } inline size_t GetCurrentAllocationCount() const override { return m_CurrentAllocationCount; } + inline size_t GetMaxedGenerationCount() const override { return 0; } inline size_t GetResetCount() const override { return m_ResetCount; } #endif private: diff --git a/SubstrateTests/Tests/TestAllocationHandle.cpp b/SubstrateTests/Tests/TestAllocationHandle.cpp index 09a71f8..0ad1195 100644 --- a/SubstrateTests/Tests/TestAllocationHandle.cpp +++ b/SubstrateTests/Tests/TestAllocationHandle.cpp @@ -75,13 +75,11 @@ TEST_CASE("AllocationHandle Generation creation and validation", "[AllocationHan REQUIRE(handle.IsValid() == true); } - SECTION("Max value mask") - { - using TestHandle16_16_0 = Substrate::DefineHandle<16, 0, uint16_t>; - TestHandle16_16_0 handle = TestHandle16_16_0(0); - REQUIRE(handle.GetGenerationMask() == 0xFFFF); - REQUIRE(handle.IsValid() == false); - } + // "Max value mask" (DefineHandle<16, 0, uint16_t>) was removed on 2026-09-01. + // An all-ones generation mask leaves zero index bits, so the handle can address + // nothing and every instance read as invalid. That is now a static_assert in + // BaseHandle -- the instantiation itself is ill-formed, so there is no object + // left to assert against from here. Do not re-add it; it will not compile. SECTION("Overflow protection") { @@ -139,13 +137,9 @@ TEST_CASE("AllocationHandle Index handling", "[AllocationHandle][Index]") REQUIRE(handle.GetMaxIndexValue() == 0xFFF); } - SECTION("Zero index mask") - { - using TestHandle16_16_0 = Substrate::DefineHandle<16, 0, uint16_t>; - TestHandle16_16_0 handle = TestHandle16_16_0(0); - REQUIRE(handle.GetIndexMask() == 0x0000); - REQUIRE(handle.IsValid() == false); - } + // "Zero index mask" removed on 2026-09-01 for the same reason as "Max value mask" + // above: it instantiated DefineHandle<16, 0, uint16_t>, which the BaseHandle + // static_assert now rejects at compile time. SECTION("Max value index mask") { diff --git a/SubstrateTests/Tests/TestPoolAllocator.cpp b/SubstrateTests/Tests/TestPoolAllocator.cpp index 51da3fa..5863921 100644 --- a/SubstrateTests/Tests/TestPoolAllocator.cpp +++ b/SubstrateTests/Tests/TestPoolAllocator.cpp @@ -4,6 +4,9 @@ #include "Substrate/PoolAllocator.h" #include "Substrate/UtilityFunctions.h" +#include +#include + struct TestStruct // 9 bytes -> aligned to 12 bytes { @@ -19,6 +22,25 @@ constexpr size_t BlockSizeWithPadding = 12 + 4; using Allocator = Substrate::PoolAllocator; +// Raw value one generation step is worth: the generation sits above the index bits, +// so incrementing it adds (1 << INDEX_BIT_COUNT) to the raw handle. +constexpr uint32_t GenerationStep = 1u << Allocator::INDEX_BIT_COUNT; // 1 << 7 == 128 + +// A deliberately tiny pool for the generation-exhaustion cases. 36 / 12 == 3 blocks, +// so INDEX_BIT_COUNT == Log2Up(3) == 2 and GENERATION_BIT_COUNT == 16 - 2 == 14. +// That is 16383 generations per slot instead of the 33.5 million a uint32_t handle +// gives, which keeps these cases in the millisecond range. +constexpr size_t SmallAllocatorSize = 3 * TestStructSize; +using SmallAllocator = Substrate::PoolAllocator; + +// 4 blocks == an exact power of two, so the highest index (3) equals the index mask +// (INDEX_BIT_COUNT == Log2Up(4) == 2). That slot used to read as invalid purely from +// its index bits, which made it allocatable but never freeable. Aurora's pools are +// this shape at 1024 blocks, so the case is worth pinning. +constexpr size_t PowerOfTwoAllocatorSize = 4 * TestStructSize; +using PowerOfTwoAllocator = Substrate::PoolAllocator; + + TEST_CASE("Pool Allocator", "[Allocator][Pool]") { SECTION("Creation") @@ -30,8 +52,9 @@ TEST_CASE("Pool Allocator", "[Allocator][Pool]") REQUIRE(allocator.GetCurrentAllocationCount() == 0); REQUIRE(allocator.GetTotalAllocationCount() == 0); REQUIRE(allocator.GetResetCount() == 0); - REQUIRE(allocator.GetFreeHandleIndices().size() == MaxAllocations); - + REQUIRE(allocator.GetMaxedGenerationCount() == 0); + REQUIRE(allocator.GetFreeHandleCount() == MaxAllocations); + REQUIRE(Allocator::MAX_BLOCK_COUNT == MaxAllocations); REQUIRE(Allocator::INDEX_BIT_COUNT == Substrate::Utility::Log2Up(85)); //should be 7 @@ -46,6 +69,7 @@ TEST_CASE("Pool Allocator", "[Allocator][Pool]") REQUIRE(allocator.GetUsedMemory() == TestStructSize); REQUIRE(allocator.GetCurrentAllocationCount() == 1); REQUIRE(allocator.GetTotalAllocationCount() == 1); + REQUIRE(allocator.GetFreeHandleCount() == MaxAllocations - 1); } SECTION("GetPointerFromHandle") @@ -79,23 +103,92 @@ TEST_CASE("Pool Allocator", "[Allocator][Pool]") REQUIRE(allocator.GetUsedMemory() == 0); REQUIRE(allocator.GetCurrentAllocationCount() == 0); REQUIRE(allocator.GetTotalAllocationCount() == 1); + REQUIRE(allocator.GetFreeHandleCount() == MaxAllocations); } - SECTION("Free and reallocate") + SECTION("Free and reallocate hands out a different slot") { Allocator allocator; uint32_t handle1 = allocator.Allocate(); + REQUIRE(handle1 == 0); // index 0, generation 0 allocator.Free(handle1); + + // FIFO: the freed index goes to the back of the queue, so the next allocation + // takes the next untouched slot instead of handing index 0 straight back. + // Under the old LIFO free list this returned 128 (index 0, generation 1). uint32_t handle2 = allocator.Allocate(); - - REQUIRE(handle1 != handle2); // Handle should have a different generation now - REQUIRE(handle2 == 128); // Handle index should be 0 again, but the first generation bit should be incremented by 1 -> 1 << 7 == 128 + REQUIRE(handle2 == 1); REQUIRE(allocator.GetUsedMemory() == TestStructSize); REQUIRE(allocator.GetCurrentAllocationCount() == 1); REQUIRE(allocator.GetTotalAllocationCount() == 2); } + SECTION("A freed slot only returns after a full rotation") + { + Allocator allocator; + uint32_t first = allocator.Allocate(); + REQUIRE(first == 0); + allocator.Free(first); // index 0 -> back of the queue, generation 1 + + // Every other slot is handed out before index 0 comes back around. + for (size_t i = 1; i < MaxAllocations; i++) + { + uint32_t handle = allocator.Allocate(); + REQUIRE(handle == static_cast(i)); // still generation 0 + } + + // The queue now holds index 0 alone, carrying generation 1. + uint32_t reused = allocator.Allocate(); + REQUIRE(reused == GenerationStep); + REQUIRE(allocator.GetFreeHandleCount() == 0); + } + + SECTION("Generation burn spreads across every slot") + { + // This is the property the FIFO free list exists for. Under LIFO this same + // loop drove index 0 to generation 85 and left every other slot at 0. + Allocator allocator; + for (size_t i = 0; i < MaxAllocations; i++) + { + uint32_t handle = allocator.Allocate(); + REQUIRE(handle == static_cast(i)); // generation 0 on the first pass + allocator.Free(handle); + } + + // One rotation later every slot sits at generation 1, none of them higher. + for (size_t i = 0; i < MaxAllocations; i++) + { + uint32_t handle = allocator.Allocate(); + REQUIRE(handle == GenerationStep + static_cast(i)); + } + REQUIRE(allocator.GetFreeHandleCount() == 0); + } + + SECTION("Reuse order survives the ring wrapping") + { + // MAX_BLOCK_COUNT is 85 here, not a power of two, so the wrap is a plain + // compare rather than a mask. Filling the pool wraps the head; freeing the + // whole pool afterwards wraps the tail. + Allocator allocator; + std::vector live; + live.reserve(MaxAllocations); + for (size_t i = 0; i < MaxAllocations; i++) + live.push_back(allocator.Allocate()); + + REQUIRE(allocator.GetFreeHandleCount() == 0); + + for (uint32_t handle : live) + allocator.Free(handle); + + REQUIRE(allocator.GetFreeHandleCount() == MaxAllocations); + REQUIRE(allocator.GetCurrentAllocationCount() == 0); + + // Freed in index order, so they must come back in index order. + for (size_t i = 0; i < MaxAllocations; i++) + REQUIRE(allocator.Allocate() == GenerationStep + static_cast(i)); + } + SECTION("Allocate until full") { Allocator allocator; @@ -112,25 +205,104 @@ TEST_CASE("Pool Allocator", "[Allocator][Pool]") REQUIRE(allocator.GetUsedMemory() == MaxAllocations * TestStructSize); REQUIRE(allocator.GetCurrentAllocationCount() == MaxAllocations); REQUIRE(allocator.GetTotalAllocationCount() == MaxAllocations); + + // The exhaustion guard has to read the free count: m_FreeHandles is a + // fixed-capacity ring now and its size() never drops to zero. + REQUIRE(allocator.GetFreeHandleCount() == 0); REQUIRE_THROWS_AS(allocator.Allocate(), Substrate::AllocatorOutOfMemoryException); } - SECTION("Allocate and reallocate until generations are exhausted") + SECTION("The top slot of a power-of-two pool is usable") { - Allocator allocator; - constexpr size_t maxGenerations = (1 << Allocator::GENERATION_BIT_COUNT) - 1; - uint32_t firstHandle; - for (size_t gen = 0; gen < maxGenerations; gen++) + // Regression cover for GAP-030. The highest index equals the index mask here, + // so a validity check that keys off "all index bits set" wrongly condemns it: + // the slot allocates, but Free takes the retirement branch and it never + // returns to the ring. Validity has to key off the sentinel instead. + PowerOfTwoAllocator allocator; + constexpr uint16_t topIndex = PowerOfTwoAllocator::MAX_BLOCK_COUNT - 1; // 3 == index mask + + std::vector handles; + for (size_t i = 0; i < PowerOfTwoAllocator::MAX_BLOCK_COUNT; i++) + handles.push_back(allocator.Allocate()); + + REQUIRE(handles.back() == topIndex); // generation 0, index 3 + REQUIRE(allocator.GetPointerFromHandle(handles.back()) != nullptr); + REQUIRE(allocator.IsHandleValid(handles.back())); + + allocator.Free(handles.back()); + REQUIRE(allocator.GetMaxedGenerationCount() == 0); // retirement must NOT have fired + REQUIRE(allocator.GetFreeHandleCount() == 1); + REQUIRE(allocator.GetCurrentAllocationCount() == PowerOfTwoAllocator::MAX_BLOCK_COUNT - 1); + + // It comes back with its generation incremented, like any other slot. + constexpr uint16_t smallGenerationStep = 1u << PowerOfTwoAllocator::INDEX_BIT_COUNT; + REQUIRE(allocator.Allocate() == static_cast(smallGenerationStep + topIndex)); + } + + SECTION("Generation exhaustion retires a single slot") + { + // Hold every slot but one. With exactly one entry in the free list, FIFO + // hands that same slot back every cycle, so its generation burns without + // needing a full rotation per step. + SmallAllocator allocator; + constexpr size_t maxGenerations = (1u << SmallAllocator::GENERATION_BIT_COUNT) - 1; + + std::vector held; + for (size_t i = 1; i < SmallAllocator::MAX_BLOCK_COUNT; i++) + held.push_back(allocator.Allocate()); + + uint16_t cycling = allocator.Allocate(); + REQUIRE(allocator.GetFreeHandleCount() == 0); + REQUIRE(allocator.GetCurrentAllocationCount() == SmallAllocator::MAX_BLOCK_COUNT); + + // Burn it up to the last usable generation. No REQUIRE inside the loop: + // Catch2 assertions cost far more than the work being exercised. + for (size_t generation = 1; generation < maxGenerations; generation++) { - firstHandle = allocator.Allocate(); - allocator.Free(firstHandle); + allocator.Free(cycling); + cycling = allocator.Allocate(); } - REQUIRE(allocator.GetTotalAllocationCount() == maxGenerations); - REQUIRE(allocator.GetCurrentAllocationCount() == 1); // Last allocation is still active because it wont get freed again - REQUIRE(allocator.GetUsedMemory() == TestStructSize); // We dont free -> used memory should still be 1 block + REQUIRE(allocator.GetCurrentAllocationCount() == SmallAllocator::MAX_BLOCK_COUNT); - // Next allocation should use the next index, as all generations for index 0 are exhausted and it will not end up in the free list again - uint32_t handle = allocator.Allocate(); - REQUIRE(handle == 1); // Next index + // The next free saturates the generation. The slot is retired: it does not + // return to the free list, so the pool is permanently one block short. + allocator.Free(cycling); + REQUIRE(allocator.GetFreeHandleCount() == 0); + REQUIRE(allocator.GetMaxedGenerationCount() == 1); + REQUIRE_THROWS_AS(allocator.Allocate(), Substrate::AllocatorOutOfMemoryException); + + // Known behaviour, not a fault in this test: a retired slot still counts as + // allocated, so the count overstates from here on. GetMaxedGenerationCount() + // is what makes that difference explainable rather than looking like a leak. + REQUIRE(allocator.GetCurrentAllocationCount() == SmallAllocator::MAX_BLOCK_COUNT); + } + + SECTION("Retiring one slot leaves the rest of the ring usable") + { + SmallAllocator allocator; + constexpr size_t maxGenerations = (1u << SmallAllocator::GENERATION_BIT_COUNT) - 1; + + std::vector held; + for (size_t i = 1; i < SmallAllocator::MAX_BLOCK_COUNT; i++) + held.push_back(allocator.Allocate()); + + uint16_t cycling = allocator.Allocate(); + for (size_t generation = 1; generation < maxGenerations; generation++) + { + allocator.Free(cycling); + cycling = allocator.Allocate(); + } + allocator.Free(cycling); // retires that slot + + // The slots held throughout still free and reallocate normally. + for (uint16_t handle : held) + allocator.Free(handle); + + REQUIRE(allocator.GetFreeHandleCount() == SmallAllocator::MAX_BLOCK_COUNT - 1); + REQUIRE(allocator.GetMaxedGenerationCount() == 1); + for (size_t i = 0; i < SmallAllocator::MAX_BLOCK_COUNT - 1; i++) + REQUIRE(allocator.GetPointerFromHandle(allocator.Allocate()) != nullptr); + + REQUIRE_THROWS_AS(allocator.Allocate(), Substrate::AllocatorOutOfMemoryException); } -} \ No newline at end of file +} diff --git a/SubstrateTests/Tests/TestRefCounted_st.cpp b/SubstrateTests/Tests/TestRefCounted_st.cpp index e272b7d..9dcedc5 100644 --- a/SubstrateTests/Tests/TestRefCounted_st.cpp +++ b/SubstrateTests/Tests/TestRefCounted_st.cpp @@ -184,4 +184,143 @@ TEST_CASE("RefCounted testing (single threaded) - if-check", "[RefCounted][if_st REQUIRE(true); else REQUIRE(false); +} + + +// --- CreateRefFromThis ------------------------------------------------------- +// CreateRefFromThis is protected, so these need a public shim the same way +// TestClass exposes DecRefPublic() for DecRef. + +class SelfRefClass : public Substrate::RefCounted +{ +public: + inline static int s_DestructorCalled = 0; + SelfRefClass() = default; + virtual ~SelfRefClass() + { + s_DestructorCalled++; + }; + + Ref SelfRef() + { + return CreateRefFromThis(); + } +}; + +// The consumer shape that surfaced the missing AddRef: a scope-local object that +// copies the returned Ref into a member. Mirrors VulkanSubmissionScheduler holding +// a Ref built from CreateRefFromThis. +class SelfRefConsumer +{ +public: + explicit SelfRefConsumer(Ref owner) : m_Owner(owner) {} +private: + Ref m_Owner; +}; + +class SelfRefBase : public Substrate::RefCounted +{ +public: + inline static int s_DestructorCalled = 0; + SelfRefBase() = default; + virtual ~SelfRefBase() + { + s_DestructorCalled++; + }; +}; + +class SelfRefDerived : public SelfRefBase +{ +public: + SelfRefDerived() = default; + + Ref SelfRef() + { + return CreateRefFromThis(); + } +}; + + +TEST_CASE("RefCounted testing (single threaded) - CreateRefFromThis acquires a count", "[RefCounted][SelfRef_st]") +{ + SelfRefClass::s_DestructorCalled = 0; + + Ref owner = CreateRef(); + REQUIRE(owner->GetRefCount() == 1); + { + Ref self = owner->SelfRef(); + REQUIRE(self.Get() == owner.Get()); + REQUIRE(owner->GetRefCount() == 2); + } + // Back to the original count, with the subject still alive: CreateRefFromThis + // hands out a reference it acquired, not one it borrowed from its owner. + REQUIRE(SelfRefClass::s_DestructorCalled == 0); + REQUIRE(owner->GetRefCount() == 1); +} + +TEST_CASE("RefCounted testing (single threaded) - CreateRefFromThis into a member", "[RefCounted][SelfRef_st]") +{ + SelfRefClass::s_DestructorCalled = 0; + + Ref owner = CreateRef(); + REQUIRE(owner->GetRefCount() == 1); + { + SelfRefConsumer consumer(owner->SelfRef()); + REQUIRE(owner->GetRefCount() == 2); + } + // Without the AddRef inside CreateRefFromThis this net-decrements: the returned + // Ref and the consumer's member both release a single acquisition, so the object + // is destroyed here while `owner` still holds it. Assert the destructor first, + // or the count read below is itself a use-after-free. + REQUIRE(SelfRefClass::s_DestructorCalled == 0); + REQUIRE(owner->GetRefCount() == 1); +} + +TEST_CASE("RefCounted testing (single threaded) - CreateRefFromThis does not drift", "[RefCounted][SelfRef_st]") +{ + SelfRefClass::s_DestructorCalled = 0; + + Ref owner = CreateRef(); + for (int i = 0; i < 100; i++) + { + SelfRefConsumer consumer(owner->SelfRef()); + } + + // Neither leaks nor over-releases across repeated use. + REQUIRE(SelfRefClass::s_DestructorCalled == 0); + REQUIRE(owner->GetRefCount() == 1); +} + +TEST_CASE("RefCounted testing (single threaded) - CreateRefFromThis upcast counts once", "[RefCounted][SelfRef_st]") +{ + SelfRefBase::s_DestructorCalled = 0; + + Ref derived = CreateRef(); + REQUIRE(derived->GetRefCount() == 1); + { + Ref base = derived->SelfRef(); + REQUIRE(base.Get() == derived.Get()); + REQUIRE(derived->GetRefCount() == 2); // once for the upcast, not twice + } + REQUIRE(SelfRefBase::s_DestructorCalled == 0); + REQUIRE(derived->GetRefCount() == 1); +} + +TEST_CASE("RefCounted testing (single threaded) - CreateRefFromThis outliving its owner", "[RefCounted][SelfRef_st]") +{ + SelfRefClass::s_DestructorCalled = 0; + + Ref self = nullptr; + { + Ref owner = CreateRef(); + self = owner->SelfRef(); + REQUIRE(owner->GetRefCount() == 2); + } + + // The self-reference is a real owner, so it keeps the object alive on its own. + REQUIRE(SelfRefClass::s_DestructorCalled == 0); + REQUIRE(self->GetRefCount() == 1); + + self = nullptr; + REQUIRE(SelfRefClass::s_DestructorCalled == 1); // destroyed exactly once } \ No newline at end of file