From 1f878ce4cd03e3b9c68c6ae71eb7183aa6b052ce Mon Sep 17 00:00:00 2001 From: Billy Laws Date: Fri, 19 Jul 2024 23:22:50 +0000 Subject: [PATCH] FixedSizePooledAllocation: Fix a race when unclaiming disowned buffers A disowned buffer could be unclaimed or claimed by a different thread in the time between the !IsFree check and locking the allocation mutex. Fix this and prevent such errors in the future by always checking ownership with the allocator locked before attempting to unclaim buffers. --- .../FEXCore/Utils/ThreadPoolAllocator.h | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/FEXCore/include/FEXCore/Utils/ThreadPoolAllocator.h b/FEXCore/include/FEXCore/Utils/ThreadPoolAllocator.h index 8a6efb38c..8bc31db8e 100644 --- a/FEXCore/include/FEXCore/Utils/ThreadPoolAllocator.h +++ b/FEXCore/include/FEXCore/Utils/ThreadPoolAllocator.h @@ -120,16 +120,16 @@ public: } /** - * @brief Immediately release the buffer back to the allocator + * @brief Immediately release the buffer back to the allocator, given it has not been reclaimed * * @param Buffer - The iterator that was previously given with ClaimBuffer - * - * Once this is called on a buffer then the pool allocator has full ownership of the buffer */ - void UnclaimBuffer(ContainerType::iterator Buffer) { - std::unique_lock lk {AllocationMutex}; - (*Buffer)->CurrentClientOwnedFlag->store(ClientFlags::FLAG_FREE); - UnclaimBufferImpl(Buffer); + void UnclaimBuffer(ContainerType::iterator Buffer, BufferOwnedFlag* ClientFlag) { + // Transition the buffer to free, unclaiming if it wasn't free prior. + if (ClientFlag->exchange(ClientFlags::FLAG_FREE) != ClientFlags::FLAG_FREE) { + std::unique_lock lk {AllocationMutex}; + UnclaimBufferImpl(Buffer); + } } /** @@ -485,9 +485,7 @@ public: * Only use in that edge case! Otherwise use `DelayedDisownBuffer` */ void UnclaimBuffer() { - if (!FEXCore::Utils::IntrusivePooledAllocator::IsClientBufferFree(ClientOwnedFlag)) { - ThreadAllocator.UnclaimBuffer(Info); - } + ThreadAllocator.UnclaimBuffer(Info, &ClientOwnedFlag); } private: