From cffbeb205606425fee6fd1d0b569a87c6fff956e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Plewa?= Date: Thu, 5 Jun 2025 14:10:12 +0200 Subject: [PATCH] unify return values for destroy functions fixes: #1217 --- .../custom_file_provider.c | 3 +- include/umf/memory_pool.h | 3 +- include/umf/memory_pool_ops.h | 3 +- include/umf/memory_provider.h | 3 +- include/umf/memory_provider_ops.h | 3 +- include/umf/memspace.h | 5 ++- include/umf/pools/pool_disjoint.h | 3 +- src/memory_pool.c | 17 +++++--- src/memory_provider.c | 15 +++++-- src/memspace.c | 5 ++- src/pool/pool_disjoint.c | 16 ++++--- src/pool/pool_jemalloc.c | 9 +++- src/pool/pool_proxy.c | 5 ++- src/pool/pool_scalable.c | 9 +++- src/provider/provider_cuda.c | 3 +- src/provider/provider_devdax_memory.c | 11 ++++- src/provider/provider_file_memory.c | 11 ++++- src/provider/provider_fixed_memory.c | 3 +- src/provider/provider_level_zero.c | 3 +- src/provider/provider_os_memory.c | 3 +- src/provider/provider_tracking.c | 3 +- test/c_api/disjoint_pool.c | 7 +++- test/coarse_lib.cpp | 3 +- test/common/pool_null.c | 5 ++- test/common/pool_trace.c | 5 ++- test/common/provider_null.c | 5 ++- test/common/provider_trace.c | 9 +++- test/disjoint_pool_file_prov.cpp | 24 +++++++---- test/memoryProviderAPI.cpp | 5 +++ test/memspaces/mempolicy.cpp | 12 ++++-- test/memspaces/memspace_numa.cpp | 36 ++++++++++------ test/provider_fixed_memory.cpp | 12 ++++-- test/provider_tracking.cpp | 42 ++++++++++++------- test/test_init_teardown.c | 4 +- test/utils/cpp_helpers.hpp | 10 ++++- 35 files changed, 223 insertions(+), 92 deletions(-) diff --git a/examples/custom_file_provider/custom_file_provider.c b/examples/custom_file_provider/custom_file_provider.c index 6454bb78f..dbf1bb092 100644 --- a/examples/custom_file_provider/custom_file_provider.c +++ b/examples/custom_file_provider/custom_file_provider.c @@ -104,11 +104,12 @@ static umf_result_t file_init(const void *params, void **provider) { } // Function to deinitialize the file provider -static void file_deinit(void *provider) { +static umf_result_t file_deinit(void *provider) { file_provider_t *file_provider = (file_provider_t *)provider; munmap(file_provider->ptr, ADDRESS_RESERVATION); close(file_provider->fd); free(file_provider); + return UMF_RESULT_SUCCESS; } // Function to allocate memory from the file provider diff --git a/include/umf/memory_pool.h b/include/umf/memory_pool.h index 1f8500f95..c9b02214e 100644 --- a/include/umf/memory_pool.h +++ b/include/umf/memory_pool.h @@ -58,8 +58,9 @@ umf_result_t umfPoolCreate(const umf_memory_pool_ops_t *ops, /// /// @brief Destroys memory pool. /// @param hPool handle to the pool +/// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure. /// -void umfPoolDestroy(umf_memory_pool_handle_t hPool); +umf_result_t umfPoolDestroy(umf_memory_pool_handle_t hPool); /// /// @brief Allocates \p size bytes of uninitialized storage from \p hPool diff --git a/include/umf/memory_pool_ops.h b/include/umf/memory_pool_ops.h index b0216bfd0..7b03ec8d2 100644 --- a/include/umf/memory_pool_ops.h +++ b/include/umf/memory_pool_ops.h @@ -47,8 +47,9 @@ typedef struct umf_memory_pool_ops_t { /// /// @brief Finalizes memory pool /// @param pool pool to finalize + /// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure. /// - void (*finalize)(void *pool); + umf_result_t (*finalize)(void *pool); /// /// @brief Allocates \p size bytes of uninitialized storage from \p pool diff --git a/include/umf/memory_provider.h b/include/umf/memory_provider.h index fb843274a..b9fbb28c9 100644 --- a/include/umf/memory_provider.h +++ b/include/umf/memory_provider.h @@ -51,8 +51,9 @@ umf_result_t umfMemoryProviderCreate(const umf_memory_provider_ops_t *ops, /// /// @brief Destroys memory provider. /// @param hProvider handle to the memory provider +/// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure. /// -void umfMemoryProviderDestroy(umf_memory_provider_handle_t hProvider); +umf_result_t umfMemoryProviderDestroy(umf_memory_provider_handle_t hProvider); /// /// @brief Allocates \p size bytes of uninitialized storage from memory \p hProvider diff --git a/include/umf/memory_provider_ops.h b/include/umf/memory_provider_ops.h index 1ee8363d9..8f383235c 100644 --- a/include/umf/memory_provider_ops.h +++ b/include/umf/memory_provider_ops.h @@ -42,8 +42,9 @@ typedef struct umf_memory_provider_ops_t { /// /// @brief Finalizes memory provider. /// @param provider provider to finalize + /// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure. /// - void (*finalize)(void *provider); + umf_result_t (*finalize)(void *provider); /// /// @brief Allocates \p size bytes of uninitialized storage from memory \p provider diff --git a/include/umf/memspace.h b/include/umf/memspace.h index 85b6b3681..4b4597ef3 100644 --- a/include/umf/memspace.h +++ b/include/umf/memspace.h @@ -1,6 +1,6 @@ /* * - * Copyright (C) 2023-2024 Intel Corporation + * Copyright (C) 2023-2025 Intel Corporation * * Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT. * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception @@ -58,8 +58,9 @@ umf_result_t umfMemspaceCreateFromNumaArray(unsigned *nodeIds, size_t numIds, /// /// \brief Destroys memspace /// \param hMemspace handle to memspace +/// \return UMF_RESULT_SUCCESS on success or appropriate error code on failure. /// -void umfMemspaceDestroy(umf_memspace_handle_t hMemspace); +umf_result_t umfMemspaceDestroy(umf_memspace_handle_t hMemspace); /// /// \brief Retrieves predefined host all memspace. diff --git a/include/umf/pools/pool_disjoint.h b/include/umf/pools/pool_disjoint.h index 640184c97..9f467d9a6 100644 --- a/include/umf/pools/pool_disjoint.h +++ b/include/umf/pools/pool_disjoint.h @@ -35,7 +35,8 @@ umfDisjointPoolSharedLimitsCreate(size_t MaxSize); /// @brief Destroy previously created pool limits struct. /// @param hSharedLimits handle to the shared limits struct. -void umfDisjointPoolSharedLimitsDestroy( +/// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure. +umf_result_t umfDisjointPoolSharedLimitsDestroy( umf_disjoint_pool_shared_limits_handle_t hSharedLimits); /// @brief Create a struct to store parameters of disjoint pool. diff --git a/src/memory_pool.c b/src/memory_pool.c index c98b677b5..b1d9664a2 100644 --- a/src/memory_pool.c +++ b/src/memory_pool.c @@ -201,24 +201,30 @@ static umf_result_t umfPoolCreateInternal(const umf_memory_pool_ops_t *ops, return ret; } -void umfPoolDestroy(umf_memory_pool_handle_t hPool) { +umf_result_t umfPoolDestroy(umf_memory_pool_handle_t hPool) { if (umf_ba_global_is_destroyed()) { - return; + return UMF_RESULT_ERROR_UNKNOWN; } - hPool->ops.finalize(hPool->pool_priv); + umf_result_t ret = hPool->ops.finalize(hPool->pool_priv); umf_memory_provider_handle_t hUpstreamProvider = NULL; umfPoolGetMemoryProvider(hPool, &hUpstreamProvider); if (!(hPool->flags & UMF_POOL_CREATE_FLAG_DISABLE_TRACKING)) { // Destroy tracking provider. - umfMemoryProviderDestroy(hPool->provider); + umf_result_t ret2 = umfMemoryProviderDestroy(hPool->provider); + if (ret == UMF_RESULT_SUCCESS) { + ret = ret2; + } } if (hPool->flags & UMF_POOL_CREATE_FLAG_OWN_PROVIDER) { // Destroy associated memory provider. - umfMemoryProviderDestroy(hUpstreamProvider); + umf_result_t ret2 = umfMemoryProviderDestroy(hUpstreamProvider); + if (ret == UMF_RESULT_SUCCESS) { + ret = ret2; + } } utils_mutex_destroy_not_free(&hPool->lock); @@ -227,6 +233,7 @@ void umfPoolDestroy(umf_memory_pool_handle_t hPool) { // TODO: this free keeps memory in base allocator, so it can lead to OOM in some scenarios (it should be optimized) umf_ba_global_free(hPool); + return ret; } umf_result_t umfFree(void *ptr) { diff --git a/src/memory_provider.c b/src/memory_provider.c index ca044b340..c262ad80d 100644 --- a/src/memory_provider.c +++ b/src/memory_provider.c @@ -235,11 +235,18 @@ umf_result_t umfMemoryProviderCreate(const umf_memory_provider_ops_t *ops, return UMF_RESULT_SUCCESS; } -void umfMemoryProviderDestroy(umf_memory_provider_handle_t hProvider) { - if (hProvider && !umf_ba_global_is_destroyed()) { - hProvider->ops.finalize(hProvider->provider_priv); - umf_ba_global_free(hProvider); +umf_result_t umfMemoryProviderDestroy(umf_memory_provider_handle_t hProvider) { + if (umf_ba_global_is_destroyed()) { + return UMF_RESULT_ERROR_UNKNOWN; } + + if (!hProvider) { + return UMF_RESULT_SUCCESS; + } + + umf_result_t ret = hProvider->ops.finalize(hProvider->provider_priv); + umf_ba_global_free(hProvider); + return ret; } static void diff --git a/src/memspace.c b/src/memspace.c index 1cd80e1fa..31b52e26f 100644 --- a/src/memspace.c +++ b/src/memspace.c @@ -1,6 +1,6 @@ /* * - * Copyright (C) 2023-2024 Intel Corporation + * Copyright (C) 2023-2025 Intel Corporation * * Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT. * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception @@ -130,7 +130,7 @@ umf_result_t umfMemspaceNew(umf_memspace_handle_t *hMemspace) { return UMF_RESULT_SUCCESS; } -void umfMemspaceDestroy(umf_memspace_handle_t memspace) { +umf_result_t umfMemspaceDestroy(umf_memspace_handle_t memspace) { assert(memspace); for (size_t i = 0; i < memspace->size; i++) { umfMemtargetDestroy(memspace->nodes[i]); @@ -138,6 +138,7 @@ void umfMemspaceDestroy(umf_memspace_handle_t memspace) { umf_ba_global_free(memspace->nodes); umf_ba_global_free(memspace); + return UMF_RESULT_SUCCESS; } umf_result_t umfMemspaceClone(umf_const_memspace_handle_t hMemspace, diff --git a/src/pool/pool_disjoint.c b/src/pool/pool_disjoint.c index 6629c9a5b..9885a6a68 100644 --- a/src/pool/pool_disjoint.c +++ b/src/pool/pool_disjoint.c @@ -995,9 +995,9 @@ umf_result_t disjoint_pool_get_last_allocation_error(void *pool) { } // Define destructor for use with unique_ptr -void disjoint_pool_finalize(void *pool) { +umf_result_t disjoint_pool_finalize(void *pool) { disjoint_pool_t *hPool = (disjoint_pool_t *)pool; - + umf_result_t ret = UMF_RESULT_SUCCESS; if (hPool->params.pool_trace > 1) { disjoint_pool_print_stats(hPool); } @@ -1007,11 +1007,16 @@ void disjoint_pool_finalize(void *pool) { } VALGRIND_DO_DESTROY_MEMPOOL(hPool); + ret = umfDisjointPoolSharedLimitsDestroy(hPool->default_shared_limits); + if (ret != UMF_RESULT_SUCCESS) { + ret = UMF_RESULT_ERROR_UNKNOWN; + LOG_ERR("umfDisjointPoolSharedLimitsDestroy failed"); + } - umfDisjointPoolSharedLimitsDestroy(hPool->default_shared_limits); critnib_delete(hPool->known_slabs); umf_ba_global_free(hPool); + return ret; } const char *disjoint_pool_get_name(void *pool) { @@ -1053,9 +1058,10 @@ umfDisjointPoolSharedLimitsCreate(size_t max_size) { return ptr; } -void umfDisjointPoolSharedLimitsDestroy( - umf_disjoint_pool_shared_limits_t *limits) { +umf_result_t +umfDisjointPoolSharedLimitsDestroy(umf_disjoint_pool_shared_limits_t *limits) { umf_ba_global_free(limits); + return UMF_RESULT_SUCCESS; } umf_result_t diff --git a/src/pool/pool_jemalloc.c b/src/pool/pool_jemalloc.c index b9d0d07c4..8d6a2daf2 100644 --- a/src/pool/pool_jemalloc.c +++ b/src/pool/pool_jemalloc.c @@ -521,18 +521,23 @@ static umf_result_t op_initialize(umf_memory_provider_handle_t provider, return UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC; } -static void op_finalize(void *pool) { +static umf_result_t op_finalize(void *pool) { assert(pool); + umf_result_t ret = UMF_RESULT_SUCCESS; jemalloc_memory_pool_t *je_pool = (jemalloc_memory_pool_t *)pool; for (size_t i = 0; i < je_pool->n_arenas; i++) { char cmd[64]; unsigned arena = je_pool->arena_index[i]; snprintf(cmd, sizeof(cmd), "arena.%u.destroy", arena); - (void)je_mallctl(cmd, NULL, 0, NULL, 0); + if (je_mallctl(cmd, NULL, 0, NULL, 0)) { + LOG_ERR("Could not destroy jemalloc arena %u", arena); + ret = UMF_RESULT_ERROR_UNKNOWN; + } } umf_ba_global_free(je_pool); VALGRIND_DO_DESTROY_MEMPOOL(pool); + return ret; } static size_t op_malloc_usable_size(void *pool, const void *ptr) { diff --git a/src/pool/pool_proxy.c b/src/pool/pool_proxy.c index 91ae098ca..208b46d4c 100644 --- a/src/pool/pool_proxy.c +++ b/src/pool/pool_proxy.c @@ -39,7 +39,10 @@ proxy_pool_initialize(umf_memory_provider_handle_t hProvider, return UMF_RESULT_SUCCESS; } -static void proxy_pool_finalize(void *pool) { umf_ba_global_free(pool); } +static umf_result_t proxy_pool_finalize(void *pool) { + umf_ba_global_free(pool); + return UMF_RESULT_SUCCESS; +} static void *proxy_aligned_malloc(void *pool, size_t size, size_t alignment) { assert(pool); diff --git a/src/pool/pool_scalable.c b/src/pool/pool_scalable.c index f626523b6..fa63351c2 100644 --- a/src/pool/pool_scalable.c +++ b/src/pool/pool_scalable.c @@ -313,10 +313,15 @@ static umf_result_t tbb_pool_initialize(umf_memory_provider_handle_t provider, return res; } -static void tbb_pool_finalize(void *pool) { +static umf_result_t tbb_pool_finalize(void *pool) { tbb_memory_pool_t *pool_data = (tbb_memory_pool_t *)pool; - tbb_callbacks.pool_destroy(pool_data->tbb_pool); + umf_result_t ret = UMF_RESULT_SUCCESS; + if (!tbb_callbacks.pool_destroy(pool_data->tbb_pool)) { + LOG_ERR("TBB pool destroy failed"); + ret = UMF_RESULT_ERROR_UNKNOWN; + } umf_ba_global_free(pool_data); + return ret; } static void *tbb_malloc(void *pool, size_t size) { diff --git a/src/provider/provider_cuda.c b/src/provider/provider_cuda.c index b69a1f6e2..9f0dc6d98 100644 --- a/src/provider/provider_cuda.c +++ b/src/provider/provider_cuda.c @@ -369,8 +369,9 @@ static umf_result_t cu_memory_provider_initialize(const void *params, return UMF_RESULT_SUCCESS; } -static void cu_memory_provider_finalize(void *provider) { +static umf_result_t cu_memory_provider_finalize(void *provider) { umf_ba_global_free(provider); + return UMF_RESULT_SUCCESS; } /* diff --git a/src/provider/provider_devdax_memory.c b/src/provider/provider_devdax_memory.c index c230798a0..fdd8ad9b9 100644 --- a/src/provider/provider_devdax_memory.c +++ b/src/provider/provider_devdax_memory.c @@ -265,12 +265,19 @@ static umf_result_t devdax_initialize(const void *params, void **provider) { return ret; } -static void devdax_finalize(void *provider) { +static umf_result_t devdax_finalize(void *provider) { devdax_memory_provider_t *devdax_provider = provider; + umf_result_t ret = UMF_RESULT_SUCCESS; utils_mutex_destroy_not_free(&devdax_provider->lock); - utils_munmap(devdax_provider->base, devdax_provider->size); + if (utils_munmap(devdax_provider->base, devdax_provider->size)) { + LOG_PERR("unmapping the devdax memory failed (path: %s, size: %zu)", + devdax_provider->path, devdax_provider->size); + ret = UMF_RESULT_ERROR_UNKNOWN; + } + coarse_delete(devdax_provider->coarse); umf_ba_global_free(devdax_provider); + return ret; } static umf_result_t devdax_alloc(void *provider, size_t size, size_t alignment, diff --git a/src/provider/provider_file_memory.c b/src/provider/provider_file_memory.c index da2216507..b4ff44f8a 100644 --- a/src/provider/provider_file_memory.c +++ b/src/provider/provider_file_memory.c @@ -305,12 +305,13 @@ static umf_result_t file_initialize(const void *params, void **provider) { return ret; } -static void file_finalize(void *provider) { +static umf_result_t file_finalize(void *provider) { file_memory_provider_t *file_provider = provider; uintptr_t key = 0; uintptr_t rkey = 0; void *rvalue = NULL; + umf_result_t ret = UMF_RESULT_SUCCESS; while (1 == critnib_find(file_provider->mmaps, key, FIND_G, &rkey, &rvalue, NULL)) { utils_munmap((void *)rkey, (size_t)rvalue); @@ -319,11 +320,17 @@ static void file_finalize(void *provider) { } utils_mutex_destroy_not_free(&file_provider->lock); - utils_close_fd(file_provider->fd); + + if (utils_close_fd(file_provider->fd)) { + LOG_PERR("closing file descriptor %d failed", file_provider->fd); + ret = UMF_RESULT_ERROR_UNKNOWN; + } critnib_delete(file_provider->fd_offset_map); critnib_delete(file_provider->mmaps); coarse_delete(file_provider->coarse); umf_ba_global_free(file_provider); + + return ret; } static umf_result_t file_mmap_aligned(file_memory_provider_t *file_provider, diff --git a/src/provider/provider_fixed_memory.c b/src/provider/provider_fixed_memory.c index 7a791f83c..c30044946 100644 --- a/src/provider/provider_fixed_memory.c +++ b/src/provider/provider_fixed_memory.c @@ -153,10 +153,11 @@ static umf_result_t fixed_initialize(const void *params, void **provider) { return ret; } -static void fixed_finalize(void *provider) { +static umf_result_t fixed_finalize(void *provider) { fixed_memory_provider_t *fixed_provider = provider; coarse_delete(fixed_provider->coarse); umf_ba_global_free(fixed_provider); + return UMF_RESULT_SUCCESS; } static umf_result_t fixed_alloc(void *provider, size_t size, size_t alignment, diff --git a/src/provider/provider_level_zero.c b/src/provider/provider_level_zero.c index c860b01f2..69092d49a 100644 --- a/src/provider/provider_level_zero.c +++ b/src/provider/provider_level_zero.c @@ -479,11 +479,12 @@ static umf_result_t query_min_page_size(ze_memory_provider_t *ze_provider, return ze2umf_result(ze_result); } -static void ze_memory_provider_finalize(void *provider) { +static umf_result_t ze_memory_provider_finalize(void *provider) { ze_memory_provider_t *ze_provider = (ze_memory_provider_t *)provider; umf_ba_global_free(ze_provider->resident_device_handles); umf_ba_global_free(provider); + return UMF_RESULT_SUCCESS; } static umf_result_t ze_memory_provider_initialize(const void *params, diff --git a/src/provider/provider_os_memory.c b/src/provider/provider_os_memory.c index a97d81bb9..a794d79b8 100644 --- a/src/provider/provider_os_memory.c +++ b/src/provider/provider_os_memory.c @@ -701,7 +701,7 @@ static umf_result_t os_initialize(const void *params, void **provider) { return ret; } -static void os_finalize(void *provider) { +static umf_result_t os_finalize(void *provider) { os_memory_provider_t *os_provider = provider; if (os_provider->fd > 0) { @@ -721,6 +721,7 @@ static void os_finalize(void *provider) { } hwloc_topology_destroy(os_provider->topo); umf_ba_global_free(os_provider); + return UMF_RESULT_SUCCESS; } // TODO: this function should be re-enabled when CTL is implemented diff --git a/src/provider/provider_tracking.c b/src/provider/provider_tracking.c index 39e6eadb2..252316ea6 100644 --- a/src/provider/provider_tracking.c +++ b/src/provider/provider_tracking.c @@ -988,7 +988,7 @@ static void check_if_tracker_is_empty(umf_memory_tracker_handle_t hTracker, } #endif /* NDEBUG */ -static void trackingFinalize(void *provider) { +static umf_result_t trackingFinalize(void *provider) { umf_tracking_memory_provider_t *p = (umf_tracking_memory_provider_t *)provider; @@ -997,6 +997,7 @@ static void trackingFinalize(void *provider) { critnib_delete(p->ipcCache); umf_ba_global_free(provider); + return UMF_RESULT_SUCCESS; } static void trackingGetLastError(void *provider, const char **msg, diff --git a/test/c_api/disjoint_pool.c b/test/c_api/disjoint_pool.c index b529497c8..905835160 100644 --- a/test/c_api/disjoint_pool.c +++ b/test/c_api/disjoint_pool.c @@ -48,8 +48,11 @@ void test_disjoint_pool_shared_limits(void) { umfPoolDestroy(pool); umfMemoryProviderDestroy(provider); - umfDisjointPoolSharedLimitsDestroy(limits); - umfDisjointPoolParamsDestroy(params); + retp = umfDisjointPoolSharedLimitsDestroy(limits); + UT_ASSERTeq(retp, UMF_RESULT_SUCCESS); + + retp = umfDisjointPoolParamsDestroy(params); + UT_ASSERTeq(retp, UMF_RESULT_SUCCESS); } int main(void) { diff --git a/test/coarse_lib.cpp b/test/coarse_lib.cpp index 069061285..ceb4cd514 100644 --- a/test/coarse_lib.cpp +++ b/test/coarse_lib.cpp @@ -168,7 +168,8 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_basic_provider) { ASSERT_EQ(coarse_get_stats(ch).num_all_blocks, (size_t)1); coarse_delete(ch); - umfMemoryProviderDestroy(malloc_memory_provider); + umf_result = umfMemoryProviderDestroy(malloc_memory_provider); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } TEST_P(CoarseWithMemoryStrategyTest, coarseTest_basic_fixed_memory) { diff --git a/test/common/pool_null.c b/test/common/pool_null.c index 3f01d865b..a44f3c4ab 100644 --- a/test/common/pool_null.c +++ b/test/common/pool_null.c @@ -17,7 +17,10 @@ static umf_result_t nullInitialize(umf_memory_provider_handle_t provider, return UMF_RESULT_SUCCESS; } -static void nullFinalize(void *pool) { (void)pool; } +static umf_result_t nullFinalize(void *pool) { + (void)pool; + return UMF_RESULT_SUCCESS; +} static void *nullMalloc(void *pool, size_t size) { (void)pool; diff --git a/test/common/pool_trace.c b/test/common/pool_trace.c index 63f33e1ef..e4479548f 100644 --- a/test/common/pool_trace.c +++ b/test/common/pool_trace.c @@ -31,7 +31,10 @@ static umf_result_t traceInitialize(umf_memory_provider_handle_t provider, return UMF_RESULT_SUCCESS; } -static void traceFinalize(void *pool) { free(pool); } +static umf_result_t traceFinalize(void *pool) { + free(pool); + return UMF_RESULT_SUCCESS; +} static void *traceMalloc(void *pool, size_t size) { trace_pool_t *trace_pool = (trace_pool_t *)pool; diff --git a/test/common/provider_null.c b/test/common/provider_null.c index 630ea75af..8c1602be7 100644 --- a/test/common/provider_null.c +++ b/test/common/provider_null.c @@ -14,7 +14,10 @@ static umf_result_t nullInitialize(const void *params, void **pool) { return UMF_RESULT_SUCCESS; } -static void nullFinalize(void *pool) { (void)pool; } +static umf_result_t nullFinalize(void *pool) { + (void)pool; + return UMF_RESULT_SUCCESS; +} static umf_result_t nullAlloc(void *provider, size_t size, size_t alignment, void **ptr) { diff --git a/test/common/provider_trace.c b/test/common/provider_trace.c index 6f5e95e0f..6be80c0de 100644 --- a/test/common/provider_trace.c +++ b/test/common/provider_trace.c @@ -28,13 +28,18 @@ static umf_result_t traceInitialize(const void *params, void **pool) { return UMF_RESULT_SUCCESS; } -static void traceFinalize(void *provider) { +static umf_result_t traceFinalize(void *provider) { umf_provider_trace_params_t *traceProvider = (umf_provider_trace_params_t *)provider; if (traceProvider->own_upstream) { - umfMemoryProviderDestroy(traceProvider->hUpstreamProvider); + umf_result_t ret = + umfMemoryProviderDestroy(traceProvider->hUpstreamProvider); + if (ret != UMF_RESULT_SUCCESS) { + return ret; + } } free(provider); + return UMF_RESULT_SUCCESS; } static umf_result_t traceAlloc(void *provider, size_t size, size_t alignment, diff --git a/test/disjoint_pool_file_prov.cpp b/test/disjoint_pool_file_prov.cpp index 58e15f571..08fad03e2 100644 --- a/test/disjoint_pool_file_prov.cpp +++ b/test/disjoint_pool_file_prov.cpp @@ -129,9 +129,12 @@ TEST_P(FileWithMemoryStrategyTest, disjointFileMallocPool_simple1) { } } - umfPoolDestroy(pool); - umfMemoryProviderDestroy(file_memory_provider); - umfMemoryProviderDestroy(malloc_memory_provider); + umf_result = umfPoolDestroy(pool); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); + umf_result = umfMemoryProviderDestroy(file_memory_provider); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); + umf_result = umfMemoryProviderDestroy(malloc_memory_provider); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } TEST_P(FileWithMemoryStrategyTest, disjointFileMallocPool_simple2) { @@ -202,9 +205,12 @@ TEST_P(FileWithMemoryStrategyTest, disjointFileMallocPool_simple2) { } } - umfPoolDestroy(pool); - umfMemoryProviderDestroy(file_memory_provider); - umfMemoryProviderDestroy(malloc_memory_provider); + umf_result = umfPoolDestroy(pool); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); + umf_result = umfMemoryProviderDestroy(file_memory_provider); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); + umf_result = umfMemoryProviderDestroy(malloc_memory_provider); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } struct alloc_ptr_size { @@ -358,6 +364,8 @@ TEST_P(FileWithMemoryStrategyTest, disjointFileMMapPool_random) { allocs.erase(allocs.begin()); } - umfPoolDestroy(pool); - umfMemoryProviderDestroy(file_memory_provider); + umf_result = umfPoolDestroy(pool); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); + umf_result = umfMemoryProviderDestroy(file_memory_provider); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } diff --git a/test/memoryProviderAPI.cpp b/test/memoryProviderAPI.cpp index 97d50a145..e35fc5282 100644 --- a/test/memoryProviderAPI.cpp +++ b/test/memoryProviderAPI.cpp @@ -169,6 +169,11 @@ TEST_F(test, memoryProviderOpsNullAllIPCFields) { umfMemoryProviderDestroy(hProvider); } +TEST_F(test, memoryProviderNullDelete) { + auto ret = umfMemoryProviderDestroy(nullptr); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); +} + ////////////////// Negative test cases ///////////////// TEST_F(test, memoryProviderCreateNullOps) { diff --git a/test/memspaces/mempolicy.cpp b/test/memspaces/mempolicy.cpp index 7b9c4891d..2cde749be 100644 --- a/test/memspaces/mempolicy.cpp +++ b/test/memspaces/mempolicy.cpp @@ -81,7 +81,8 @@ TEST_F(test, mempolicyDefaultInterleave) { EXPECT_EQ(ProviderInternal->numa_flags, HWLOC_MEMBIND_BYNODESET); EXPECT_EQ(ProviderInternal->part_size, 0); EXPECT_EQ(ProviderInternal->mode, UMF_NUMA_MODE_INTERLEAVE); - umfMemoryProviderDestroy(hProvider); + ret = umfMemoryProviderDestroy(hProvider); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); } TEST_F(test, mempolicyInterleavePartSize) { @@ -109,7 +110,8 @@ TEST_F(test, mempolicyInterleavePartSize) { HWLOC_MEMBIND_BYNODESET | HWLOC_MEMBIND_STRICT); EXPECT_EQ(ProviderInternal->part_size, part_size); EXPECT_EQ(ProviderInternal->mode, UMF_NUMA_MODE_INTERLEAVE); - umfMemoryProviderDestroy(hProvider); + ret = umfMemoryProviderDestroy(hProvider); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); } TEST_F(test, mempolicyDefaultSplit) { @@ -134,7 +136,8 @@ TEST_F(test, mempolicyDefaultSplit) { HWLOC_MEMBIND_BYNODESET | HWLOC_MEMBIND_STRICT); EXPECT_EQ(ProviderInternal->partitions_len, ProviderInternal->nodeset_len); EXPECT_EQ(ProviderInternal->mode, UMF_NUMA_MODE_SPLIT); - umfMemoryProviderDestroy(hProvider); + ret = umfMemoryProviderDestroy(hProvider); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); } TEST_F(test, mempolicyCustomSplit) { @@ -166,7 +169,8 @@ TEST_F(test, mempolicyCustomSplit) { EXPECT_EQ(ProviderInternal->partitions_weight_sum, 2); EXPECT_EQ(ProviderInternal->partitions[0].target, ProviderInternal->partitions[1].target); - umfMemoryProviderDestroy(hProvider); + ret = umfMemoryProviderDestroy(hProvider); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); } TEST_F(test, mempolicySplitNegative) { diff --git a/test/memspaces/memspace_numa.cpp b/test/memspaces/memspace_numa.cpp index 83c8bfaf3..be961fb89 100644 --- a/test/memspaces/memspace_numa.cpp +++ b/test/memspaces/memspace_numa.cpp @@ -71,7 +71,8 @@ TEST_F(numaNodesTest, createDestroy) { EXPECT_NE(umfMemspaceMemtargetGet(hMemspace, i), nullptr); } - umfMemspaceDestroy(hMemspace); + ret = umfMemspaceDestroy(hMemspace); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); } TEST_F(numaNodesTest, createInvalidNullArray) { @@ -102,7 +103,8 @@ TEST_F(memspaceNumaTest, providerFromNumaMemspace) { ASSERT_EQ(ret, UMF_RESULT_SUCCESS); ASSERT_NE(hProvider, nullptr); - umfMemoryProviderDestroy(hProvider); + ret = umfMemoryProviderDestroy(hProvider); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); } TEST_F(memspaceNumaTest, memtargetsInvalid) { @@ -157,9 +159,12 @@ TEST_F(memspaceNumaTest, memspaceCopyTarget) { ret = umfMemoryProviderFree(hProvider2, ptr2, SIZE_4K); ASSERT_EQ(ret, UMF_RESULT_SUCCESS); - umfMemoryProviderDestroy(hProvider1); - umfMemoryProviderDestroy(hProvider2); - umfMemspaceDestroy(hMemspaceCopy); + ret = umfMemoryProviderDestroy(hProvider1); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); + ret = umfMemoryProviderDestroy(hProvider2); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); + ret = umfMemspaceDestroy(hMemspaceCopy); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); } TEST_F(memspaceNumaTest, memspaceDeleteTarget) { @@ -216,9 +221,12 @@ TEST_F(memspaceNumaTest, memspaceDeleteTarget) { ret = umfMemoryProviderFree(hProvider2, ptr2, SIZE_4K); ASSERT_EQ(ret, UMF_RESULT_SUCCESS); - umfMemoryProviderDestroy(hProvider1); - umfMemoryProviderDestroy(hProvider2); - umfMemspaceDestroy(hMemspaceCopy); + ret = umfMemoryProviderDestroy(hProvider1); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); + ret = umfMemoryProviderDestroy(hProvider2); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); + ret = umfMemspaceDestroy(hMemspaceCopy); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); } TEST_F(memspaceNumaProviderTest, allocFree) { @@ -277,7 +285,8 @@ TEST_F(numaNodesCapacityTest, CapacityFilter) { ASSERT_LT(capacity, filter_size); } - umfMemspaceDestroy(hMemspace); + ret = umfMemspaceDestroy(hMemspace); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); } TEST_F(numaNodesTest, idfilter) { @@ -308,7 +317,8 @@ TEST_F(numaNodesTest, idfilter) { ids.erase(it); } } - umfMemspaceDestroy(hMemspace); + ret = umfMemspaceDestroy(hMemspace); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); } int customfilter(umf_const_memspace_handle_t memspace, @@ -356,7 +366,8 @@ TEST_F(numaNodesTest, customfilter) { } } ASSERT_EQ(vec.size(), 0); - umfMemspaceDestroy(hMemspace); + ret = umfMemspaceDestroy(hMemspace); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); } int invalidFilter(umf_const_memspace_handle_t memspace, @@ -393,5 +404,6 @@ TEST_F(numaNodesTest, invalidFilters) { ret = umfMemspaceUserFilter(hMemspace, invalidFilter, nullptr); ASSERT_EQ(ret, UMF_RESULT_ERROR_USER_SPECIFIC); - umfMemspaceDestroy(hMemspace); + ret = umfMemspaceDestroy(hMemspace); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); } diff --git a/test/provider_fixed_memory.cpp b/test/provider_fixed_memory.cpp index d9bfc1f70..e47a4d3e7 100644 --- a/test/provider_fixed_memory.cpp +++ b/test/provider_fixed_memory.cpp @@ -438,14 +438,16 @@ TEST_P(FixedProviderTest, pool_from_ptr_whole_size_success) { umf_result = umfPoolFree(poolFromPtr, ptr); ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); - umfPoolDestroy(poolFromPtr); + umf_result = umfPoolDestroy(poolFromPtr); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); umfMemoryProviderDestroy(providerFromPtr); umfFixedMemoryProviderParamsDestroy(params); umf_result = umfPoolFree(proxyFixedPool, ptr_for_pool); ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); - umfPoolDestroy(proxyFixedPool); + umf_result = umfPoolDestroy(proxyFixedPool); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } TEST_P(FixedProviderTest, pool_from_ptr_half_size_success) { @@ -491,12 +493,14 @@ TEST_P(FixedProviderTest, pool_from_ptr_half_size_success) { umf_result = umfPoolFree(poolFromPtr, ptr); ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); - umfPoolDestroy(poolFromPtr); + umf_result = umfPoolDestroy(poolFromPtr); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); umfMemoryProviderDestroy(providerFromPtr); umfFixedMemoryProviderParamsDestroy(params); umf_result = umfPoolFree(proxyFixedPool, ptr_for_pool); ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); - umfPoolDestroy(proxyFixedPool); + umf_result = umfPoolDestroy(proxyFixedPool); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } diff --git a/test/provider_tracking.cpp b/test/provider_tracking.cpp index a0ae9955f..b41ff6012 100644 --- a/test/provider_tracking.cpp +++ b/test/provider_tracking.cpp @@ -152,8 +152,10 @@ TEST_P(TrackingProviderTest, whole_size_success) { umf_result = umfPoolFree(pool1, ptr1); ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); - umfPoolDestroy(pool1); - umfMemoryProviderDestroy(provider1); + umf_result = umfPoolDestroy(pool1); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); + umf_result = umfMemoryProviderDestroy(provider1); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); umf_result = umfPoolFree(pool0, ptr0); ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); @@ -184,8 +186,10 @@ TEST_P(TrackingProviderTest, half_size_success) { umf_result = umfPoolFree(pool1, ptr1); ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); - umfPoolDestroy(pool1); - umfMemoryProviderDestroy(provider1); + umf_result = umfPoolDestroy(pool1); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); + umf_result = umfMemoryProviderDestroy(provider1); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); umf_result = umfPoolFree(pool0, ptr0); ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); @@ -213,8 +217,10 @@ TEST_P(TrackingProviderTest, failure_exceeding_size) { ptr1 = umfPoolMalloc(pool1, size1); ASSERT_EQ(ptr1, nullptr); - umfPoolDestroy(pool1); - umfMemoryProviderDestroy(provider1); + umf_result = umfPoolDestroy(pool1); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); + umf_result = umfMemoryProviderDestroy(provider1); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); umf_result = umfPoolFree(pool0, ptr0); ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); @@ -253,8 +259,10 @@ TEST_P(TrackingProviderTest, success_max_levels) { ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); for (int i = TEST_LEVEL_SUCCESS - 1; i >= 0; i--) { - umfPoolDestroy(pools[i + 1]); - umfMemoryProviderDestroy(providers[i + 1]); + umf_result = umfPoolDestroy(pools[i + 1]); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); + umf_result = umfMemoryProviderDestroy(providers[i + 1]); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); fprintf(stderr, "Free #%d\n", i); umf_result = umfPoolFree(pools[i], ptr[i]); @@ -288,8 +296,10 @@ TEST_P(TrackingProviderTest, failure_exceeding_levels) { ASSERT_EQ(ptr[f], nullptr); for (int i = TEST_LEVEL_FAILURE - 1; i >= 0; i--) { - umfPoolDestroy(pools[i + 1]); - umfMemoryProviderDestroy(providers[i + 1]); + umf_result = umfPoolDestroy(pools[i + 1]); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); + umf_result = umfMemoryProviderDestroy(providers[i + 1]); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); fprintf(stderr, "Free #%d\n", i); umf_result = umfPoolFree(pools[i], ptr[i]); @@ -327,8 +337,10 @@ TEST_P(TrackingProviderTest, reverted_free_half_size) { umf_result = umfPoolFree(pool1, ptr1); ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); - umfPoolDestroy(pool1); - umfMemoryProviderDestroy(provider1); + umf_result = umfPoolDestroy(pool1); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); + umf_result = umfMemoryProviderDestroy(provider1); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); // It could have been freed above, // so we cannot verify the result here. @@ -366,8 +378,10 @@ TEST_P(TrackingProviderTest, reverted_free_the_same_size) { umf_result = umfPoolFree(pool1, ptr1); ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); - umfPoolDestroy(pool1); - umfMemoryProviderDestroy(provider1); + umf_result = umfPoolDestroy(pool1); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); + umf_result = umfMemoryProviderDestroy(provider1); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); // It could have been freed above, // so we cannot verify the result here. diff --git a/test/test_init_teardown.c b/test/test_init_teardown.c index e6141c135..14409bbfc 100644 --- a/test/test_init_teardown.c +++ b/test/test_init_teardown.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2024 Intel Corporation + * Copyright (C) 2024-2025 Intel Corporation * * Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT. * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception @@ -18,7 +18,7 @@ typedef int (*umfMemoryProviderCreateFromMemspace_t)(void *hMemspace, void **hPool); typedef int (*umfPoolCreate_t)(void *ops, void *provider, void *params, uint32_t flags, void **hPool); -typedef void (*umfDestroy_t)(void *handle); +typedef int (*umfDestroy_t)(void *handle); typedef void (*umfVoidVoid_t)(void); typedef void *(*umfGetPtr_t)(void); diff --git a/test/utils/cpp_helpers.hpp b/test/utils/cpp_helpers.hpp index 8b3a77517..ae18d7b1b 100644 --- a/test/utils/cpp_helpers.hpp +++ b/test/utils/cpp_helpers.hpp @@ -68,7 +68,10 @@ umf_result_t initialize(T *obj, ArgsTuple &&args) { template umf_memory_pool_ops_t poolOpsBase() { umf_memory_pool_ops_t ops{}; ops.version = UMF_POOL_OPS_VERSION_CURRENT; - ops.finalize = [](void *obj) { delete reinterpret_cast(obj); }; + ops.finalize = [](void *obj) { + delete reinterpret_cast(obj); + return UMF_RESULT_SUCCESS; + }; UMF_ASSIGN_OP(ops, T, malloc, ((void *)nullptr)); UMF_ASSIGN_OP(ops, T, calloc, ((void *)nullptr)); UMF_ASSIGN_OP(ops, T, aligned_malloc, ((void *)nullptr)); @@ -82,7 +85,10 @@ template umf_memory_pool_ops_t poolOpsBase() { template constexpr umf_memory_provider_ops_t providerOpsBase() { umf_memory_provider_ops_t ops{}; ops.version = UMF_PROVIDER_OPS_VERSION_CURRENT; - ops.finalize = [](void *obj) { delete reinterpret_cast(obj); }; + ops.finalize = [](void *obj) { + delete reinterpret_cast(obj); + return UMF_RESULT_SUCCESS; + }; UMF_ASSIGN_OP(ops, T, alloc, UMF_RESULT_ERROR_UNKNOWN); UMF_ASSIGN_OP(ops, T, free, UMF_RESULT_ERROR_UNKNOWN); UMF_ASSIGN_OP_NORETURN(ops, T, get_last_native_error);