unified-memory-framework
unified-memory-framework copied to clipboard
The pool_proxy causes memory leak when used with OS memory provider.
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. Theproxy_pool
can request the page size during the initialization and automatically round up the size inside theproxy_malloc()
function.
@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);
}
I propose a fix: #482
New fix: https://github.com/oneapi-src/unified-memory-framework/pull/487