unified-memory-framework icon indicating copy to clipboard operation
unified-memory-framework copied to clipboard

The pool_proxy causes memory leak when used with OS memory provider.

Open vinser52 opened this issue 9 months ago • 3 comments

The pool_proxy implementation is straightforward and simply proxies all allocation/deallocation requests to the underlying memory provider. The proxy_free() function just calls the umfMemoryProviderFree() function which requires the size of the allocation as the last argument. Since the proxy_pool implementation has no any state, the size is unknown and the proxy_free() function just passes 0 as the size argument to the umfMemoryProviderFree() function.

Furthermore, OS memory provider implementation ignores this fact. When 0 is passed as the size argument to the os_free it ignores the error returned by the os_munmap function if the size is 0.

How often bug is revealed:

always

Details

  • We can use a memory tracker to get the size of the allocations when free is called.
  • Change the OS memory provider implementation to not ignore if the size is 0 during deallocation.
  • Valgrind does not check mmap/munmap by default. We should add instrumentation to the os memory provider.
  • Also the proxy_pool should support arbitrary allocations size. Today the size should be multiple of the minimum page size supported by the underlying memory provider. The proxy_pool can request the page size during the initialization and automatically round up the size inside the proxy_malloc() function.

vinser52 avatar May 08 '24 12:05 vinser52

@vinser52 @igchor @bratpiorka

Furthermore, OS memory provider implementation ignores this fact. When 0 is passed as the size argument to the os_free it ignores the error returned by the os_munmap function if the size is 0.

os_free() should not ignore this error, but it cannot be changed now, because of the implementation of disjoint pool (see: #481): https://github.com/oneapi-src/unified-memory-framework/blob/6fc3e56b4870184dec70613359c4db2ef7f1180a/src/pool/pool_disjoint.cpp#L405-L411

and because of the implementation of proxy_free() of course:

static umf_result_t (void *pool, void *ptr) {
    assert(pool);
    struct proxy_memory_pool *hPool = (struct proxy_memory_pool *)pool;
    return umfMemoryProviderFree(hPool->hProvider, ptr, 0);
}

ldorau avatar May 10 '24 07:05 ldorau

I propose a fix: #482

ldorau avatar May 10 '24 08:05 ldorau

New fix: https://github.com/oneapi-src/unified-memory-framework/pull/487

ldorau avatar May 10 '24 12:05 ldorau