Merge pull request #806 from ldorau/Add_parameter_to_destroy_upstream_memory_provider_in_finalize

Add parameter to destroy upstream_memory_provider in finalize()

Author: ldorau

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)

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);
 }
 

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<char[]> 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;