Ensure that BO handle is closed only once

When one process had exported and then opened IPC handle
of memory, then close function was called twice for the
same BO handle. It caused debugBreak() and aborted
an application.

This change allows multiple separate BOs to share one
handle. The last shared handle owner calls close() function.

Related-To: NEO-7200
Signed-off-by: Wrobel, Patryk <patryk.wrobel@intel.com>

shared/source/os_interface/linux/drm_buffer_object.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018-2022 Intel Corporation
+ * Copyright (C) 2018-2023 Intel Corporation
  *
  * SPDX-License-Identifier: MIT
  *
@@ -27,7 +27,62 @@
 
 namespace NEO {
 
-BufferObject::BufferObject(Drm *drm, uint64_t patIndex, int handle, size_t size, size_t maxOsContextCount) : drm(drm), refCount(1), handle(handle), size(size) {
+BufferObjectHandleWrapper BufferObjectHandleWrapper::acquireSharedOwnership() {
+    if (controlBlock == nullptr) {
+        controlBlock = new ControlBlock{1, 0};
+    }
+
+    std::lock_guard lock{controlBlock->blockMutex};
+    controlBlock->refCount++;
+
+    return BufferObjectHandleWrapper{boHandle, Ownership::Strong, controlBlock};
+}
+
+BufferObjectHandleWrapper BufferObjectHandleWrapper::acquireWeakOwnership() {
+    if (controlBlock == nullptr) {
+        controlBlock = new ControlBlock{1, 0};
+    }
+
+    std::lock_guard lock{controlBlock->blockMutex};
+    controlBlock->weakRefCount++;
+
+    return BufferObjectHandleWrapper{boHandle, Ownership::Weak, controlBlock};
+}
+
+BufferObjectHandleWrapper::~BufferObjectHandleWrapper() {
+    if (controlBlock == nullptr) {
+        return;
+    }
+
+    std::unique_lock lock{controlBlock->blockMutex};
+
+    if (ownership == Ownership::Strong) {
+        controlBlock->refCount--;
+    } else {
+        controlBlock->weakRefCount--;
+    }
+
+    if (controlBlock->refCount == 0 && controlBlock->weakRefCount == 0) {
+        lock.unlock();
+        delete controlBlock;
+    }
+}
+
+bool BufferObjectHandleWrapper::canCloseBoHandle() {
+    if (controlBlock == nullptr) {
+        return true;
+    }
+
+    std::lock_guard lock{controlBlock->blockMutex};
+    return controlBlock->refCount == 1;
+}
+
+BufferObject::BufferObject(Drm *drm, uint64_t patIndex, int handle, size_t size, size_t maxOsContextCount)
+    : BufferObject(drm, patIndex, BufferObjectHandleWrapper{handle}, size, maxOsContextCount) {}
+
+BufferObject::BufferObject(Drm *drm, uint64_t patIndex, BufferObjectHandleWrapper &&handle, size_t size, size_t maxOsContextCount)
+    : drm(drm), refCount(1), handle(std::move(handle)), size(size) {
+
     auto ioctlHelper = drm->getIoctlHelper();
     this->tilingMode = ioctlHelper->getDrmParamValue(DrmParam::TilingNone);
     this->lockedAddress = nullptr;
@@ -58,10 +113,15 @@ void BufferObject::setAddress(uint64_t address) {
 }
 
 bool BufferObject::close() {
-    GemClose close{};
-    close.handle = this->handle;
+    if (!this->handle.canCloseBoHandle()) {
+        PRINT_DEBUG_STRING(DebugManager.flags.PrintBOCreateDestroyResult.get(), stdout, "Skipped closing BO-%d - more shared users!\n", this->handle.getBoHandle());
+        return true;
+    }
 
-    PRINT_DEBUG_STRING(DebugManager.flags.PrintBOCreateDestroyResult.get(), stdout, "Calling gem close on handle: BO-%d\n", this->handle);
+    GemClose close{};
+    close.handle = this->handle.getBoHandle();
+
+    PRINT_DEBUG_STRING(DebugManager.flags.PrintBOCreateDestroyResult.get(), stdout, "Calling gem close on handle: BO-%d\n", this->handle.getBoHandle());
 
     auto ioctlHelper = this->drm->getIoctlHelper();
     int ret = ioctlHelper->ioctl(DrmIoctl::GemClose, &close);
@@ -72,7 +132,7 @@ bool BufferObject::close() {
         return false;
     }
 
-    this->handle = -1;
+    this->handle.setBoHandle(-1);
 
     return true;
 }
@@ -82,7 +142,7 @@ int BufferObject::wait(int64_t timeoutNs) {
         return 0;
     }
 
-    int ret = this->drm->waitHandle(this->handle, -1);
+    int ret = this->drm->waitHandle(this->handle.getBoHandle(), -1);
     UNRECOVERABLE_IF(ret != 0);
 
     return ret;
@@ -94,7 +154,7 @@ bool BufferObject::setTiling(uint32_t mode, uint32_t stride) {
     }
 
     GemSetTiling setTiling{};
-    setTiling.handle = this->handle;
+    setTiling.handle = this->handle.getBoHandle();
     setTiling.tilingMode = mode;
     setTiling.stride = stride;
     auto ioctlHelper = this->drm->getIoctlHelper();
@@ -116,7 +176,7 @@ void BufferObject::fillExecObject(ExecObject &execObject, OsContext *osContext,
     const auto osContextId = drm->isPerContextVMRequired() ? osContext->getContextId() : 0;
 
     auto ioctlHelper = drm->getIoctlHelper();
-    ioctlHelper->fillExecObject(execObject, this->handle, this->gpuAddress, drmContextId, this->bindInfo[osContextId][vmHandleId], this->isMarkedForCapture());
+    ioctlHelper->fillExecObject(execObject, this->handle.getBoHandle(), this->gpuAddress, drmContextId, this->bindInfo[osContextId][vmHandleId], this->isMarkedForCapture());
 }
 
 int BufferObject::exec(uint32_t used, size_t startOffset, unsigned int flags, bool requiresCoherency, OsContext *osContext, uint32_t vmHandleId, uint32_t drmContextId,
@@ -179,19 +239,19 @@ void BufferObject::printBOBindingResult(OsContext *osContext, uint32_t vmHandleI
     if (retVal == 0) {
         if (bind) {
             PRINT_DEBUG_STRING(DebugManager.flags.PrintBOBindingResult.get(), stdout, "bind BO-%d to VM %u, drmVmId = %u, range: %llx - %llx, size: %lld, result: %d\n",
-                               this->handle, vmHandleId, static_cast<const OsContextLinux *>(osContext)->getDrmVmIds().size() ? static_cast<const OsContextLinux *>(osContext)->getDrmVmIds()[vmHandleId] : 0, this->gpuAddress, ptrOffset(this->gpuAddress, this->size), this->size, retVal);
+                               this->handle.getBoHandle(), vmHandleId, static_cast<const OsContextLinux *>(osContext)->getDrmVmIds().size() ? static_cast<const OsContextLinux *>(osContext)->getDrmVmIds()[vmHandleId] : 0, this->gpuAddress, ptrOffset(this->gpuAddress, this->size), this->size, retVal);
         } else {
             PRINT_DEBUG_STRING(DebugManager.flags.PrintBOBindingResult.get(), stdout, "unbind BO-%d from VM %u, drmVmId = %u, range: %llx - %llx, size: %lld, result: %d\n",
-                               this->handle, vmHandleId, static_cast<const OsContextLinux *>(osContext)->getDrmVmIds().size() ? static_cast<const OsContextLinux *>(osContext)->getDrmVmIds()[vmHandleId] : 0, this->gpuAddress, ptrOffset(this->gpuAddress, this->size), this->size, retVal);
+                               this->handle.getBoHandle(), vmHandleId, static_cast<const OsContextLinux *>(osContext)->getDrmVmIds().size() ? static_cast<const OsContextLinux *>(osContext)->getDrmVmIds()[vmHandleId] : 0, this->gpuAddress, ptrOffset(this->gpuAddress, this->size), this->size, retVal);
         }
     } else {
         auto err = this->drm->getErrno();
         if (bind) {
             PRINT_DEBUG_STRING(DebugManager.flags.PrintBOBindingResult.get(), stderr, "bind BO-%d to VM %u, drmVmId = %u, range: %llx - %llx, size: %lld, result: %d, errno: %d(%s)\n",
-                               this->handle, vmHandleId, static_cast<const OsContextLinux *>(osContext)->getDrmVmIds().size() ? static_cast<const OsContextLinux *>(osContext)->getDrmVmIds()[vmHandleId] : 0, this->gpuAddress, ptrOffset(this->gpuAddress, this->size), this->size, retVal, err, strerror(err));
+                               this->handle.getBoHandle(), vmHandleId, static_cast<const OsContextLinux *>(osContext)->getDrmVmIds().size() ? static_cast<const OsContextLinux *>(osContext)->getDrmVmIds()[vmHandleId] : 0, this->gpuAddress, ptrOffset(this->gpuAddress, this->size), this->size, retVal, err, strerror(err));
         } else {
             PRINT_DEBUG_STRING(DebugManager.flags.PrintBOBindingResult.get(), stderr, "unbind BO-%d from VM %u, drmVmId = %u, range: %llx - %llx, size: %lld, result: %d, errno: %d(%s)\n",
-                               this->handle, vmHandleId, static_cast<const OsContextLinux *>(osContext)->getDrmVmIds().size() ? static_cast<const OsContextLinux *>(osContext)->getDrmVmIds()[vmHandleId] : 0, this->gpuAddress, ptrOffset(this->gpuAddress, this->size), this->size, retVal, err, strerror(err));
+                               this->handle.getBoHandle(), vmHandleId, static_cast<const OsContextLinux *>(osContext)->getDrmVmIds().size() ? static_cast<const OsContextLinux *>(osContext)->getDrmVmIds()[vmHandleId] : 0, this->gpuAddress, ptrOffset(this->gpuAddress, this->size), this->size, retVal, err, strerror(err));
         }
     }
 }

shared/source/os_interface/linux/drm_buffer_object.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2018-2022 Intel Corporation
+ * Copyright (C) 2018-2023 Intel Corporation
  *
  * SPDX-License-Identifier: MIT
  *
@@ -18,7 +18,9 @@
 #include <array>
 #include <atomic>
 #include <cstddef>
+#include <mutex>
 #include <stdint.h>
+#include <utility>
 #include <vector>
 
 namespace NEO {
@@ -29,9 +31,59 @@ class DrmMemoryManager;
 class Drm;
 class OsContext;
 
+class BufferObjectHandleWrapper {
+  private:
+    struct ControlBlock {
+        int refCount{0};
+        int weakRefCount{0};
+        std::mutex blockMutex{};
+    };
+
+    enum class Ownership : std::uint8_t {
+        Weak = 0,
+        Strong = 1,
+    };
+
+  public:
+    explicit BufferObjectHandleWrapper(int boHandle) noexcept
+        : boHandle{boHandle} {}
+
+    BufferObjectHandleWrapper(BufferObjectHandleWrapper &&other) noexcept
+        : boHandle(std::exchange(other.boHandle, -1)), ownership(other.ownership), controlBlock(std::exchange(other.controlBlock, nullptr)) {}
+
+    ~BufferObjectHandleWrapper();
+
+    BufferObjectHandleWrapper(const BufferObjectHandleWrapper &) = delete;
+    BufferObjectHandleWrapper &operator=(const BufferObjectHandleWrapper &) = delete;
+    BufferObjectHandleWrapper &operator=(BufferObjectHandleWrapper &&) = delete;
+
+    BufferObjectHandleWrapper acquireSharedOwnership();
+    BufferObjectHandleWrapper acquireWeakOwnership();
+
+    bool canCloseBoHandle();
+
+    int getBoHandle() const {
+        return boHandle;
+    }
+
+    void setBoHandle(int handle) {
+        boHandle = handle;
+    }
+
+  protected:
+    BufferObjectHandleWrapper(int boHandle, Ownership ownership, ControlBlock *controlBlock)
+        : boHandle{boHandle}, ownership{ownership}, controlBlock{controlBlock} {}
+
+    int boHandle{};
+    Ownership ownership{Ownership::Strong};
+    ControlBlock *controlBlock{};
+};
+
 class BufferObject {
   public:
     BufferObject(Drm *drm, uint64_t patIndex, int handle, size_t size, size_t maxOsContextCount);
+    BufferObject(Drm *drm, uint64_t patIndex, BufferObjectHandleWrapper &&handle, size_t size, size_t maxOsContextCount);
+
     MOCKABLE_VIRTUAL ~BufferObject() = default;
 
     struct Deleter {
@@ -65,8 +117,26 @@ class BufferObject {
     }
     uint32_t getRefCount() const;
 
+    bool isBoHandleShared() const {
+        return boHandleShared;
+    }
+
+    void markAsSharedBoHandle() {
+        boHandleShared = true;
+    }
+
+    BufferObjectHandleWrapper acquireSharedOwnershipOfBoHandle() {
+        markAsSharedBoHandle();
+        return handle.acquireSharedOwnership();
+    }
+
+    BufferObjectHandleWrapper acquireWeakOwnershipOfBoHandle() {
+        markAsSharedBoHandle();
+        return handle.acquireWeakOwnership();
+    }
+
     size_t peekSize() const { return size; }
-    int peekHandle() const { return handle; }
+    int peekHandle() const { return handle.getBoHandle(); }
     const Drm *peekDrm() const { return drm; }
     uint64_t peekAddress() const { return gpuAddress; }
     void setAddress(uint64_t address);
@@ -105,7 +175,7 @@ class BufferObject {
         return rootDeviceIndex;
     }
     int getHandle() {
-        return handle;
+        return handle.getBoHandle();
     }
 
     void setCacheRegion(CacheRegion regionIndex) { cacheRegion = regionIndex; }
@@ -152,9 +222,10 @@ class BufferObject {
     std::atomic<uint32_t> refCount;
 
     uint32_t rootDeviceIndex = std::numeric_limits<uint32_t>::max();
-    int handle; // i915 gem object handle
+    BufferObjectHandleWrapper handle; // i915 gem object handle
     uint64_t size;
     bool isReused = false;
+    bool boHandleShared = false;
 
     uint32_t tilingMode;
     bool allowCapture = false;

shared/source/os_interface/linux/drm_memory_manager.cpp

@@ -189,7 +189,7 @@ uint32_t DrmMemoryManager::unreference(NEO::BufferObject *bo, bool synchronousDe
     }
 
     std::unique_lock<std::mutex> lock(mtx, std::defer_lock);
-    if (bo->peekIsReusableAllocation()) {
+    if (bo->peekIsReusableAllocation() || bo->isBoHandleShared()) {
         lock.lock();
     }
 
@@ -200,8 +200,14 @@ uint32_t DrmMemoryManager::unreference(NEO::BufferObject *bo, bool synchronousDe
             eraseSharedBufferObject(bo);
         }
 
+        int boHandle = bo->getHandle();
         bo->close();
 
+        if (bo->isBoHandleShared() && bo->getHandle() != boHandle) {
+            // Shared BO was closed - handle was invalidated. Remove weak reference from container.
+            eraseSharedBoHandleWrapper(boHandle);
+        }
+
         if (lock) {
             lock.unlock();
         }
@@ -795,7 +801,7 @@ GraphicsAllocation *DrmMemoryManager::createGraphicsAllocationFromMultipleShared
             return nullptr;
         }
 
-        auto boHandle = openFd.handle;
+        auto boHandle = static_cast<int>(openFd.handle);
         BufferObject *bo = nullptr;
         if (reuseSharedAllocation) {
             bo = findAndReferenceSharedBufferObject(boHandle, properties.rootDeviceIndex);
@@ -808,6 +814,7 @@ GraphicsAllocation *DrmMemoryManager::createGraphicsAllocationFromMultipleShared
             totalSize += size;
 
             auto patIndex = drm.getPatIndex(nullptr, properties.allocationType, CacheRegion::Default, CachePolicy::WriteBack, false);
+            auto boHandleWrapper = reuseSharedAllocation ? BufferObjectHandleWrapper{boHandle} : tryToGetBoHandleWrapperWithSharedOwnership(boHandle);
 
             bo = new (std::nothrow) BufferObject(&drm, patIndex, boHandle, size, maxOsContextCount);
             bo->setRootDeviceIndex(properties.rootDeviceIndex);
@@ -822,7 +829,9 @@ GraphicsAllocation *DrmMemoryManager::createGraphicsAllocationFromMultipleShared
     auto heapIndex = prefer57bitAddressing ? HeapIndex::HEAP_EXTENDED : HeapIndex::HEAP_STANDARD2MB;
     auto gpuRange = acquireGpuRange(totalSize, properties.rootDeviceIndex, heapIndex);
 
-    lock.unlock();
+    if (reuseSharedAllocation) {
+        lock.unlock();
+    }
 
     AllocationData allocationData;
     properties.size = totalSize;
@@ -862,9 +871,55 @@ GraphicsAllocation *DrmMemoryManager::createGraphicsAllocationFromMultipleShared
         drmAllocation->getBufferObjectToModify(i) = bo;
     }
 
+    if (!reuseSharedAllocation) {
+        registerSharedBoHandleAllocation(drmAllocation);
+    }
+
     return drmAllocation;
 }
 
+void DrmMemoryManager::registerIpcExportedAllocation(GraphicsAllocation *graphicsAllocation) {
+    std::lock_guard lock{mtx};
+    registerSharedBoHandleAllocation(static_cast<DrmAllocation *>(graphicsAllocation));
+}
+
+void DrmMemoryManager::registerSharedBoHandleAllocation(DrmAllocation *drmAllocation) {
+    if (!drmAllocation) {
+        return;
+    }
+
+    auto &bos = drmAllocation->getBOs();
+
+    for (auto *bo : bos) {
+        if (bo == nullptr) {
+            continue;
+        }
+
+        auto foundHandleWrapperIt = sharedBoHandles.find(bo->getHandle());
+        if (foundHandleWrapperIt == std::end(sharedBoHandles)) {
+            sharedBoHandles.emplace(bo->getHandle(), bo->acquireWeakOwnershipOfBoHandle());
+        } else {
+            bo->markAsSharedBoHandle();
+        }
+    }
+}
+
+BufferObjectHandleWrapper DrmMemoryManager::tryToGetBoHandleWrapperWithSharedOwnership(int boHandle) {
+    auto foundHandleWrapperIt = sharedBoHandles.find(boHandle);
+    if (foundHandleWrapperIt == std::end(sharedBoHandles)) {
+        return BufferObjectHandleWrapper{boHandle};
+    }
+
+    return foundHandleWrapperIt->second.acquireSharedOwnership();
+}
+
+void DrmMemoryManager::eraseSharedBoHandleWrapper(int boHandle) {
+    auto foundHandleWrapperIt = sharedBoHandles.find(boHandle);
+    if (foundHandleWrapperIt != std::end(sharedBoHandles)) {
+        sharedBoHandles.erase(foundHandleWrapperIt);
+    }
+}
+
 GraphicsAllocation *DrmMemoryManager::createGraphicsAllocationFromSharedHandle(osHandle handle,
                                                                                const AllocationProperties &properties,
                                                                                bool requireSpecificBitness,
@@ -891,7 +946,7 @@ GraphicsAllocation *DrmMemoryManager::createGraphicsAllocationFromSharedHandle(o
         return nullptr;
     }
 
-    auto boHandle = openFd.handle;
+    auto boHandle = static_cast<int>(openFd.handle);
     BufferObject *bo = nullptr;
     if (reuseSharedAllocation) {
         bo = findAndReferenceSharedBufferObject(boHandle, properties.rootDeviceIndex);
@@ -901,8 +956,9 @@ GraphicsAllocation *DrmMemoryManager::createGraphicsAllocationFromSharedHandle(o
         size_t size = lseekFunction(handle, 0, SEEK_END);
 
         auto patIndex = drm.getPatIndex(nullptr, properties.allocationType, CacheRegion::Default, CachePolicy::WriteBack, false);
+        auto boHandleWrapper = reuseSharedAllocation ? BufferObjectHandleWrapper{boHandle} : tryToGetBoHandleWrapperWithSharedOwnership(boHandle);
 
-        bo = new (std::nothrow) BufferObject(&drm, patIndex, boHandle, size, maxOsContextCount);
+        bo = new (std::nothrow) BufferObject(&drm, patIndex, std::move(boHandleWrapper), size, maxOsContextCount);
 
         if (!bo) {
             return nullptr;
@@ -943,7 +999,9 @@ GraphicsAllocation *DrmMemoryManager::createGraphicsAllocationFromSharedHandle(o
         pushSharedBufferObject(bo);
     }
 
-    lock.unlock();
+    if (reuseSharedAllocation) {
+        lock.unlock();
+    }
 
     auto gmmHelper = getGmmHelper(properties.rootDeviceIndex);
     auto canonizedGpuAddress = gmmHelper->canonize(castToUint64(reinterpret_cast<void *>(bo->peekAddress())));
@@ -976,6 +1034,11 @@ GraphicsAllocation *DrmMemoryManager::createGraphicsAllocationFromSharedHandle(o
 
         bo->setPatIndex(drm.getPatIndex(gmm, properties.allocationType, CacheRegion::Default, CachePolicy::WriteBack, false));
     }
+
+    if (!reuseSharedAllocation) {
+        registerSharedBoHandleAllocation(drmAllocation);
+    }
+
     return drmAllocation;
 }
 

shared/source/os_interface/linux/drm_memory_manager.h

@@ -7,6 +7,7 @@
 
 #pragma once
 #include "shared/source/memory_manager/memory_manager.h"
+#include "shared/source/os_interface/linux/drm_buffer_object.h"
 
 #include <limits>
 #include <map>
@@ -53,6 +54,8 @@ class DrmMemoryManager : public MemoryManager {
     // drm/i915 ioctl wrappers
     MOCKABLE_VIRTUAL uint32_t unreference(BufferObject *bo, bool synchronousDestroy);
 
+    void registerIpcExportedAllocation(GraphicsAllocation *graphicsAllocation) override;
+
     bool isValidateHostMemoryEnabled() const {
         return validateHostPtrMemory;
     }
@@ -87,6 +90,10 @@ class DrmMemoryManager : public MemoryManager {
     bool allowIndirectAllocationsAsPack(uint32_t rootDeviceIndex) override;
 
   protected:
+    void registerSharedBoHandleAllocation(DrmAllocation *drmAllocation);
+    BufferObjectHandleWrapper tryToGetBoHandleWrapperWithSharedOwnership(int boHandle);
+    void eraseSharedBoHandleWrapper(int boHandle);
+
     MOCKABLE_VIRTUAL BufferObject *findAndReferenceSharedBufferObject(int boHandle, uint32_t rootDeviceIndex);
     void eraseSharedBufferObject(BufferObject *bo);
     void pushSharedBufferObject(BufferObject *bo);
@@ -151,6 +158,7 @@ class DrmMemoryManager : public MemoryManager {
     std::vector<BufferObject *> sharingBufferObjects;
     std::mutex mtx;
 
+    std::map<int, BufferObjectHandleWrapper> sharedBoHandles;
     std::vector<std::vector<GraphicsAllocation *>> localMemAllocs;
     std::vector<GraphicsAllocation *> sysMemAllocs;
     std::mutex allocMutex;