diff --git a/src/ipc_cache.c b/src/ipc_cache.c index ccb296d5bf..cab5fc478d 100644 --- a/src/ipc_cache.c +++ b/src/ipc_cache.c @@ -144,6 +144,8 @@ umfIpcOpenedCacheCreate(ipc_opened_cache_eviction_cb_t eviction_cb) { void umfIpcOpenedCacheDestroy(ipc_opened_cache_handle_t cache) { ipc_opened_cache_entry_t *entry, *tmp; + + utils_mutex_lock(&(cache->global->cache_lock)); HASH_ITER(hh, cache->hash_table, entry, tmp) { DL_DELETE(cache->global->lru_list, entry); HASH_DEL(cache->hash_table, entry); @@ -153,6 +155,7 @@ void umfIpcOpenedCacheDestroy(ipc_opened_cache_handle_t cache) { umf_ba_free(cache->global->cache_allocator, entry); } HASH_CLEAR(hh, cache->hash_table); + utils_mutex_unlock(&(cache->global->cache_lock)); umf_ba_global_free(cache); } diff --git a/src/provider/provider_tracking.c b/src/provider/provider_tracking.c index 73a03fb2da..62145d5d77 100644 --- a/src/provider/provider_tracking.c +++ b/src/provider/provider_tracking.c @@ -473,10 +473,6 @@ static void trackingFinalize(void *provider) { critnib_delete(p->ipcCache); -#ifndef NDEBUG - check_if_tracker_is_empty(p->hTracker, p->pool); -#endif /* NDEBUG */ - umf_ba_global_free(provider); } diff --git a/test/ipcFixtures.hpp b/test/ipcFixtures.hpp index cfe58a1665..23f15a63f0 100644 --- a/test/ipcFixtures.hpp +++ b/test/ipcFixtures.hpp @@ -593,4 +593,73 @@ TEST_P(umfIpcTest, ConcurrentOpenCloseHandles) { EXPECT_EQ(stat.openCount, stat.closeCount); } +TEST_P(umfIpcTest, ConcurrentDestroyIpcHandlers) { + constexpr size_t SIZE = 100; + constexpr size_t NUM_ALLOCS = 100; + constexpr size_t NUM_POOLS = 10; + void *ptrs[NUM_ALLOCS]; + void *openedPtrs[NUM_POOLS][NUM_ALLOCS]; + std::vector consumerPools; + umf::pool_unique_handle_t producerPool = makePool(); + ASSERT_NE(producerPool.get(), nullptr); + + for (size_t i = 0; i < NUM_POOLS; ++i) { + consumerPools.push_back(makePool()); + } + + for (size_t i = 0; i < NUM_ALLOCS; ++i) { + void *ptr = umfPoolMalloc(producerPool.get(), SIZE); + ASSERT_NE(ptr, nullptr); + ptrs[i] = ptr; + } + + for (size_t i = 0; i < NUM_ALLOCS; ++i) { + umf_ipc_handle_t ipcHandle = nullptr; + size_t handleSize = 0; + umf_result_t ret = umfGetIPCHandle(ptrs[i], &ipcHandle, &handleSize); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); + + for (size_t poolId = 0; poolId < NUM_POOLS; poolId++) { + void *ptr = nullptr; + umf_ipc_handler_handle_t ipcHandler = nullptr; + ret = + umfPoolGetIPCHandler(consumerPools[poolId].get(), &ipcHandler); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); + ASSERT_NE(ipcHandler, nullptr); + + ret = umfOpenIPCHandle(ipcHandler, ipcHandle, &ptr); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); + openedPtrs[poolId][i] = ptr; + } + + ret = umfPutIPCHandle(ipcHandle); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); + } + + for (size_t poolId = 0; poolId < NUM_POOLS; poolId++) { + for (size_t i = 0; i < NUM_ALLOCS; ++i) { + umf_result_t ret = umfCloseIPCHandle(openedPtrs[poolId][i]); + EXPECT_EQ(ret, UMF_RESULT_SUCCESS); + } + } + + for (size_t i = 0; i < NUM_ALLOCS; ++i) { + umf_result_t ret = umfFree(ptrs[i]); + EXPECT_EQ(ret, UMF_RESULT_SUCCESS); + } + + // Destroy pools in parallel to cause IPC cache cleanup in parallel. + umf_test::syncthreads_barrier syncthreads(NUM_POOLS); + auto poolDestroyFn = [&consumerPools, &syncthreads](size_t tid) { + syncthreads(); + consumerPools[tid].reset(nullptr); + }; + umf_test::parallel_exec(NUM_POOLS, poolDestroyFn); + + producerPool.reset(nullptr); + + EXPECT_EQ(stat.putCount, stat.getCount); + EXPECT_EQ(stat.openCount, stat.closeCount); +} + #endif /* UMF_TEST_IPC_FIXTURES_HPP */ diff --git a/test/supp/drd-umf_test-provider_devdax_memory_ipc.supp b/test/supp/drd-umf_test-provider_devdax_memory_ipc.supp index cd44bb49ae..025834658f 100644 --- a/test/supp/drd-umf_test-provider_devdax_memory_ipc.supp +++ b/test/supp/drd-umf_test-provider_devdax_memory_ipc.supp @@ -6,3 +6,20 @@ fun:umfOpenIPCHandle ... } + +{ + False-positive ConflictingAccess in jemalloc + drd:ConflictingAccess + fun:atomic_* + ... + fun:je_* + ... +} + +{ + False-positive ConflictingAccess in tbbmalloc + drd:ConflictingAccess + ... + fun:tbb_pool_finalize + ... +} diff --git a/test/supp/drd-umf_test-provider_file_memory_ipc.supp b/test/supp/drd-umf_test-provider_file_memory_ipc.supp index 7fce241167..a15d860aa5 100644 --- a/test/supp/drd-umf_test-provider_file_memory_ipc.supp +++ b/test/supp/drd-umf_test-provider_file_memory_ipc.supp @@ -14,3 +14,20 @@ fun:umfOpenIPCHandle ... } + +{ + False-positive ConflictingAccess in jemalloc + drd:ConflictingAccess + fun:atomic_* + ... + fun:je_* + ... +} + +{ + False-positive ConflictingAccess in tbbmalloc + drd:ConflictingAccess + ... + fun:tbb_pool_finalize + ... +} diff --git a/test/supp/drd-umf_test-provider_os_memory.supp b/test/supp/drd-umf_test-provider_os_memory.supp index cd44bb49ae..025834658f 100644 --- a/test/supp/drd-umf_test-provider_os_memory.supp +++ b/test/supp/drd-umf_test-provider_os_memory.supp @@ -6,3 +6,20 @@ fun:umfOpenIPCHandle ... } + +{ + False-positive ConflictingAccess in jemalloc + drd:ConflictingAccess + fun:atomic_* + ... + fun:je_* + ... +} + +{ + False-positive ConflictingAccess in tbbmalloc + drd:ConflictingAccess + ... + fun:tbb_pool_finalize + ... +} diff --git a/test/supp/helgrind-umf_test-provider_devdax_memory_ipc.supp b/test/supp/helgrind-umf_test-provider_devdax_memory_ipc.supp index 4fcd2786cf..d6401e8ee7 100644 --- a/test/supp/helgrind-umf_test-provider_devdax_memory_ipc.supp +++ b/test/supp/helgrind-umf_test-provider_devdax_memory_ipc.supp @@ -6,3 +6,20 @@ fun:umfOpenIPCHandle ... } + +{ + False-positive ConflictingAccess in jemalloc + Helgrind:Race + fun:atomic_* + ... + fun:je_* + ... +} + +{ + False-positive ConflictingAccess in tbbmalloc + Helgrind:Race + ... + fun:tbb_pool_finalize + ... +} diff --git a/test/supp/helgrind-umf_test-provider_file_memory_ipc.supp b/test/supp/helgrind-umf_test-provider_file_memory_ipc.supp index 4194f48479..cdc0bd8df4 100644 --- a/test/supp/helgrind-umf_test-provider_file_memory_ipc.supp +++ b/test/supp/helgrind-umf_test-provider_file_memory_ipc.supp @@ -23,3 +23,20 @@ fun:critnib_find ... } + +{ + False-positive ConflictingAccess in jemalloc + Helgrind:Race + fun:atomic_* + ... + fun:je_* + ... +} + +{ + False-positive ConflictingAccess in tbbmalloc + Helgrind:Race + ... + fun:tbb_pool_finalize + ... +} diff --git a/test/supp/helgrind-umf_test-provider_os_memory.supp b/test/supp/helgrind-umf_test-provider_os_memory.supp index 4fcd2786cf..d6401e8ee7 100644 --- a/test/supp/helgrind-umf_test-provider_os_memory.supp +++ b/test/supp/helgrind-umf_test-provider_os_memory.supp @@ -6,3 +6,20 @@ fun:umfOpenIPCHandle ... } + +{ + False-positive ConflictingAccess in jemalloc + Helgrind:Race + fun:atomic_* + ... + fun:je_* + ... +} + +{ + False-positive ConflictingAccess in tbbmalloc + Helgrind:Race + ... + fun:tbb_pool_finalize + ... +}