Add parameter to destroy upstream_memory_provider in finalize()

[ldorau]

Description

Add parameter to destroy upstream_memory_provider in finalize().

Checklist

• Code compiles without errors locally
• All tests pass locally
• CI workflows execute properly

[bratpiorka]

why do we need this parameter? If the user creates an upstream provider, why can't he destroy it?

[ldorau]

why do we need this parameter? If the user creates an upstream provider, why can't he destroy it?

In order to prevent a memory leak in our tests using the umfPoolTest fixture (see: #808) the upstream provider has to be destroyed automatically.

[igchor]

why do we need this parameter? If the user creates an upstream provider, why can't he destroy it?

In order to prevent a memory leak in our tests using the umfPoolTest fixture (see: #808) the upstream provider has to be destroyed automatically.

If it's only for tests then you can prevent the leak by just destroying the provider in pool_unique_handle_t. You just need to modify this line:

unified-memory-framework/test/poolFixtures.hpp

Line 53 in e802af8

return umf::pool_unique_handle_t(hPool, &umfPoolDestroy);

To be something like this:

auto dtor = [coarse_provider ](umf_memory_pool_handle_t pool) {
    umfPoolDestory(pool):
    umfMemoryProviderDestory(coarse_provider );
};
return umf::pool_unique_handle_t(hPool, std;:move(dtor));

[ldorau]

why do we need this parameter? If the user creates an upstream provider, why can't he destroy it?

In order to prevent a memory leak in our tests using the umfPoolTest fixture (see: #808) the upstream provider has to be destroyed automatically.

If it's only for tests then you can prevent the leak by just destroying the provider in pool_unique_handle_t. You just need to modify this line:

unified-memory-framework/test/poolFixtures.hpp

Line 53 in e802af8

return umf::pool_unique_handle_t(hPool, &umfPoolDestroy);

To be something like this:

auto dtor = [coarse_provider ](umf_memory_pool_handle_t pool) {
    umfPoolDestory(pool):
    umfMemoryProviderDestory(coarse_provider );
};
return umf::pool_unique_handle_t(hPool, std;:move(dtor));

Thanks @igchor ! I applied that in #808.

[vinser52]

why do we need this parameter? If the user creates an upstream provider, why can't he destroy it?

@igchor @bratpiorka @ldorau

But what if the user creates a memory pool with the UMF_POOL_CREATE_FLAG_OWN_PROVIDER flag? In that case, the user expects that the pool destructor will destroy the memory provider used by the pool. And in case the coarse provider is used, it is destroyed automatically, but the upstream memory provider is not.

Did I miss something?

[ldorau]

why do we need this parameter? If the user creates an upstream provider, why can't he destroy it?

@igchor @bratpiorka @ldorau

But what if the user creates a memory pool with the UMF_POOL_CREATE_FLAG_OWN_PROVIDER flag? In that case, the user expects that the pool destructor will destroy the memory provider used by the pool. And in case the coarse provider is used, it is destroyed automatically, but the upstream memory provider is not.

Did I miss something?

@vinser52 You are right. It seems this parameter is needed when UMF_POOL_CREATE_FLAG_OWN_PROVIDER is used.
@bratpiorka @lplewa Do you agree?

[bratpiorka]

@ldorau agree + please add test for this (for both cases - when UMF_POOL_CREATE_FLAG_OWN_PROVIDER is set to true and false)

[ldorau]

@ldorau agree + please add test for this (for both cases - when UMF_POOL_CREATE_FLAG_OWN_PROVIDER is set to true and false)

@bratpiorka Done

[lukaszstolarczuk]

👍

[vinser52]

Should the coarse provider always require the upstream provider to be specified?

What is the reason for not specifying the upstream provider? @bratpiorka what do you think?

P.S. I understand that it is out of scope for this particular PR.

[ldorau]

The coarse provider contains the fixed provider yet - in the case of the fixed provider the upstream provider is not required. The fixed provider will be moved to a separate provider and the upstream provider will be always required then.