From a1339ee108aa4f58cf8dafa18b0d1c5f219aa6ec Mon Sep 17 00:00:00 2001 From: "Mi, Yanfeng" Date: Fri, 31 Oct 2025 11:20:49 +0800 Subject: [PATCH] Refactor poolbuffer check to fix refcount issue when using clang compiler The issue root-caused to the subtle difference of unique_ptr's destructor between clang and gcc: Clang (libc++): Calls unique_ptr's destructor, which first clears the internal pointer to nullptr and then calls the deleter on the old pointer value. This means that within the destructor of the managed object, the owning unique_ptr's internal pointer is already nullified. GCC (libstdc++): The destructor directly calls the deleter on the current pointer without first setting it to nullptr Fix: using flag-based system to replace the problematic pointer comparison ref #744 Signed-off-by: Mi, Yanfeng --- opencl/source/context/context.cpp | 2 ++ opencl/source/mem_obj/buffer.h | 3 +++ shared/source/memory_manager/graphics_allocation.h | 4 ++++ shared/source/utilities/buffer_pool_allocator.inl | 3 ++- shared/source/utilities/generic_pool_allocator.inl | 3 +++ shared/source/utilities/isa_pool_allocator.cpp | 3 +++ 6 files changed, 17 insertions(+), 1 deletion(-) diff --git a/opencl/source/context/context.cpp b/opencl/source/context/context.cpp index ad2e55d37b131..df5bd4aff8972 100644 --- a/opencl/source/context/context.cpp +++ b/opencl/source/context/context.cpp @@ -574,6 +574,7 @@ Context::BufferPool::BufferPool(Context *context) : BaseType(context->memoryMana bufferCreateArgs, errcodeRet)); if (this->mainStorage) { + this->mainStorage->setAsPoolBuffer(true); this->chunkAllocator.reset(new HeapAllocator(params.startingOffset, context->getBufferPoolAllocator().getParams().aggregatedSmallBuffersPoolSize, context->getBufferPoolAllocator().getParams().chunkAlignment)); @@ -602,6 +603,7 @@ Buffer *Context::BufferPool::allocate(const MemoryProperties &memoryProperties, auto bufferFromPool = this->mainStorage->createSubBuffer(flags, flagsIntel, &bufferRegion, errcodeRet); bufferFromPool->createFunction = this->mainStorage->createFunction; bufferFromPool->setSizeInPoolAllocator(actualSize); + bufferFromPool->setAsPoolBuffer(true); return bufferFromPool; } diff --git a/opencl/source/mem_obj/buffer.h b/opencl/source/mem_obj/buffer.h index 7a5f0a463da26..ac935fb91879d 100644 --- a/opencl/source/mem_obj/buffer.h +++ b/opencl/source/mem_obj/buffer.h @@ -66,6 +66,7 @@ class Buffer : public MemObj { constexpr static cl_ulong maskMagic = 0xFFFFFFFFFFFFFFFFLL; constexpr static cl_ulong objectMagic = MemObj::objectMagic | 0x02; bool forceDisallowCPUCopy = false; + bool poolBuffer = false; ~Buffer() override; @@ -158,6 +159,8 @@ class Buffer : public MemObj { BufferCreateFunc createFunction = nullptr; bool isSubBuffer(); bool isValidSubBufferOffset(size_t offset); + void setAsPoolBuffer(bool value) { this->poolBuffer = value; } + bool isPoolBuffer() const { return poolBuffer; } uint64_t setArgStateless(void *memory, uint32_t patchSize, uint32_t rootDeviceIndex, bool set32BitAddressing); virtual void setArgStateful(void *memory, bool forceNonAuxMode, bool disableL3, bool alignSizeForAuxTranslation, bool isReadOnly, const Device &device, bool areMultipleSubDevicesInContext) = 0; diff --git a/shared/source/memory_manager/graphics_allocation.h b/shared/source/memory_manager/graphics_allocation.h index 0dc2b2eef0e44..3ba43806dd436 100644 --- a/shared/source/memory_manager/graphics_allocation.h +++ b/shared/source/memory_manager/graphics_allocation.h @@ -313,6 +313,9 @@ class GraphicsAllocation : public IDNode, NEO::NonCopyableAn bool isAllocatedInLocalMemoryPool() const { return (this->memoryPool == MemoryPool::localMemory); } bool isAllocationLockable() const; + void setAsPoolBuffer(bool value) { this->poolBuffer = value; } + bool isPoolBuffer() const { return poolBuffer; } + const AubInfo &getAubInfo() const { return aubInfo; } bool isCompressionEnabled() const; @@ -438,6 +441,7 @@ class GraphicsAllocation : public IDNode, NEO::NonCopyableAn std::atomic registeredContextsNum{0}; bool shareableHostMemory = false; bool cantBeReadOnly = false; + bool poolBuffer = false; bool explicitlyMadeResident = false; bool isImported = false; }; diff --git a/shared/source/utilities/buffer_pool_allocator.inl b/shared/source/utilities/buffer_pool_allocator.inl index 7f90961bfe321..ff6d39fcff471 100644 --- a/shared/source/utilities/buffer_pool_allocator.inl +++ b/shared/source/utilities/buffer_pool_allocator.inl @@ -48,7 +48,8 @@ template bool AbstractBuffersPool::isPoolBuffer(const BufferParentType *buffer) const { static_assert(std::is_base_of_v); - return (buffer && this->mainStorage.get() == buffer); + const auto *bufferObj = static_cast(buffer); + return bufferObj && bufferObj->isPoolBuffer(); } template diff --git a/shared/source/utilities/generic_pool_allocator.inl b/shared/source/utilities/generic_pool_allocator.inl index cc993656f18a1..db4d572b6a7eb 100644 --- a/shared/source/utilities/generic_pool_allocator.inl +++ b/shared/source/utilities/generic_pool_allocator.inl @@ -28,6 +28,9 @@ GenericPool::GenericPool(Device *device, size_t poolSize) MemoryConstants::pageSize, 0u)); this->mainStorage.reset(graphicsAllocation); + if (this->mainStorage) { + this->mainStorage->setAsPoolBuffer(true); + } this->mtx = std::make_unique(); stackVec.push_back(graphicsAllocation); } diff --git a/shared/source/utilities/isa_pool_allocator.cpp b/shared/source/utilities/isa_pool_allocator.cpp index bd9f16b4a15f1..cab918279a0ef 100644 --- a/shared/source/utilities/isa_pool_allocator.cpp +++ b/shared/source/utilities/isa_pool_allocator.cpp @@ -28,6 +28,9 @@ ISAPool::ISAPool(Device *device, bool isBuiltin, size_t storageSize) MemoryConstants::pageSize, 0u)); this->mainStorage.reset(graphicsAllocation); + if (this->mainStorage) { + this->mainStorage->setAsPoolBuffer(true); + } this->mtx = std::make_unique(); this->stackVec.push_back(graphicsAllocation); }