diff --git a/include/umf/providers/provider_coarse.h b/include/umf/providers/provider_coarse.h index 16d11fd77a..22b59a61bf 100644 --- a/include/umf/providers/provider_coarse.h +++ b/include/umf/providers/provider_coarse.h @@ -69,6 +69,9 @@ typedef struct coarse_memory_provider_params_t { /// the init_buffer is always used instead /// (regardless of the value of this parameter). bool immediate_init_from_upstream; + + /// Destroy upstream_memory_provider in finalize(). + bool destroy_upstream_memory_provider; } coarse_memory_provider_params_t; /// @brief Coarse Memory Provider stats (TODO move to CTL) diff --git a/src/provider/provider_coarse.c b/src/provider/provider_coarse.c index 872d994be8..800463d8ae 100644 --- a/src/provider/provider_coarse.c +++ b/src/provider/provider_coarse.c @@ -31,6 +31,9 @@ typedef struct coarse_memory_provider_t { umf_memory_provider_handle_t upstream_memory_provider; + // destroy upstream_memory_provider in finalize() + bool destroy_upstream_memory_provider; + // memory allocation strategy coarse_memory_provider_strategy_t allocation_strategy; @@ -898,6 +901,13 @@ static umf_result_t coarse_memory_provider_initialize(void *params, return UMF_RESULT_ERROR_INVALID_ARGUMENT; } + if (coarse_params->destroy_upstream_memory_provider && + !coarse_params->upstream_memory_provider) { + LOG_ERR("destroy_upstream_memory_provider is true, but an upstream " + "provider is not provided"); + return UMF_RESULT_ERROR_INVALID_ARGUMENT; + } + coarse_memory_provider_t *coarse_provider = umf_ba_global_alloc(sizeof(*coarse_provider)); if (!coarse_provider) { @@ -909,6 +919,8 @@ static umf_result_t coarse_memory_provider_initialize(void *params, coarse_provider->upstream_memory_provider = coarse_params->upstream_memory_provider; + coarse_provider->destroy_upstream_memory_provider = + coarse_params->destroy_upstream_memory_provider; coarse_provider->allocation_strategy = coarse_params->allocation_strategy; coarse_provider->init_buffer = coarse_params->init_buffer; @@ -1081,6 +1093,11 @@ static void coarse_memory_provider_finalize(void *provider) { umf_ba_global_free(coarse_provider->name); + if (coarse_provider->destroy_upstream_memory_provider && + coarse_provider->upstream_memory_provider) { + umfMemoryProviderDestroy(coarse_provider->upstream_memory_provider); + } + umf_ba_global_free(coarse_provider); } diff --git a/test/disjointCoarseMallocPool.cpp b/test/disjointCoarseMallocPool.cpp index b3eb3f91b8..fcb3238ee1 100644 --- a/test/disjointCoarseMallocPool.cpp +++ b/test/disjointCoarseMallocPool.cpp @@ -59,6 +59,7 @@ TEST_F(test, disjointCoarseMallocPool_name_upstream) { sizeof(coarse_memory_provider_params)); coarse_memory_provider_params.upstream_memory_provider = malloc_memory_provider; + coarse_memory_provider_params.destroy_upstream_memory_provider = true; coarse_memory_provider_params.immediate_init_from_upstream = true; coarse_memory_provider_params.init_buffer = nullptr; coarse_memory_provider_params.init_buffer_size = init_buffer_size; @@ -75,7 +76,9 @@ TEST_F(test, disjointCoarseMallocPool_name_upstream) { 0); umfMemoryProviderDestroy(coarse_memory_provider); - umfMemoryProviderDestroy(malloc_memory_provider); + // malloc_memory_provider has already been destroyed + // by umfMemoryProviderDestroy(coarse_memory_provider), because: + // coarse_memory_provider_params.destroy_upstream_memory_provider = true; } TEST_F(test, disjointCoarseMallocPool_name_no_upstream) { @@ -129,6 +132,7 @@ TEST_P(CoarseWithMemoryStrategyTest, disjointCoarseMallocPool_basic) { coarse_memory_provider_params.allocation_strategy = allocation_strategy; coarse_memory_provider_params.upstream_memory_provider = malloc_memory_provider; + coarse_memory_provider_params.destroy_upstream_memory_provider = true; coarse_memory_provider_params.immediate_init_from_upstream = true; coarse_memory_provider_params.init_buffer = nullptr; coarse_memory_provider_params.init_buffer_size = init_buffer_size; @@ -150,7 +154,7 @@ TEST_P(CoarseWithMemoryStrategyTest, disjointCoarseMallocPool_basic) { umf_memory_pool_handle_t pool; umf_result = umfPoolCreate(umfDisjointPoolOps(), coarse_memory_provider, &disjoint_memory_pool_params, - UMF_POOL_CREATE_FLAG_NONE, &pool); + UMF_POOL_CREATE_FLAG_OWN_PROVIDER, &pool); ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); ASSERT_NE(pool, nullptr); @@ -282,8 +286,10 @@ TEST_P(CoarseWithMemoryStrategyTest, disjointCoarseMallocPool_basic) { ASSERT_EQ(GetStats(prov).alloc_size, init_buffer_size); umfPoolDestroy(pool); - umfMemoryProviderDestroy(coarse_memory_provider); - umfMemoryProviderDestroy(malloc_memory_provider); + // Both coarse_memory_provider and malloc_memory_provider + // have already been destroyed by umfPoolDestroy(), because: + // UMF_POOL_CREATE_FLAG_OWN_PROVIDER was set in umfPoolCreate() and + // coarse_memory_provider_params.destroy_upstream_memory_provider = true; } TEST_P(CoarseWithMemoryStrategyTest, disjointCoarseMallocPool_simple1) { @@ -760,6 +766,37 @@ TEST_P(CoarseWithMemoryStrategyTest, disjointCoarseMallocPool_wrong_params_4) { umfMemoryProviderDestroy(malloc_memory_provider); } +// wrong parameters: destroy_upstream_memory_provider is true, but an upstream provider is not provided +TEST_P(CoarseWithMemoryStrategyTest, disjointCoarseMallocPool_wrong_params_5) { + umf_result_t umf_result; + + const size_t init_buffer_size = 20 * MB; + + // Preallocate some memory + std::unique_ptr buffer(new char[init_buffer_size]); + void *buf = buffer.get(); + ASSERT_NE(buf, nullptr); + memset(buf, 0, init_buffer_size); + + coarse_memory_provider_params_t coarse_memory_provider_params; + // make sure there are no undefined members - prevent a UB + memset(&coarse_memory_provider_params, 0, + sizeof(coarse_memory_provider_params)); + coarse_memory_provider_params.allocation_strategy = allocation_strategy; + coarse_memory_provider_params.upstream_memory_provider = nullptr; + coarse_memory_provider_params.destroy_upstream_memory_provider = true; + coarse_memory_provider_params.immediate_init_from_upstream = false; + coarse_memory_provider_params.init_buffer = buf; + coarse_memory_provider_params.init_buffer_size = init_buffer_size; + + umf_memory_provider_handle_t coarse_memory_provider = nullptr; + umf_result = umfMemoryProviderCreate(umfCoarseMemoryProviderOps(), + &coarse_memory_provider_params, + &coarse_memory_provider); + ASSERT_EQ(umf_result, UMF_RESULT_ERROR_INVALID_ARGUMENT); + ASSERT_EQ(coarse_memory_provider, nullptr); +} + TEST_P(CoarseWithMemoryStrategyTest, disjointCoarseMallocPool_split_merge) { umf_memory_provider_handle_t malloc_memory_provider; umf_result_t umf_result;