From e14f3a204179a2364f0788ac4964246a6e77bce6 Mon Sep 17 00:00:00 2001 From: "Boyarinov, Konstantin" Date: Wed, 8 Jan 2025 08:53:51 -0800 Subject: [PATCH 1/3] Add fix for memory pools mismatch for critical tasks --- include/oneapi/tbb/detail/_flow_graph_impl.h | 3 ++- src/tbb/small_object_pool.cpp | 1 + test/tbb/test_function_node.cpp | 22 ++++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/include/oneapi/tbb/detail/_flow_graph_impl.h b/include/oneapi/tbb/detail/_flow_graph_impl.h index 55063b93e1..75ed58c186 100644 --- a/include/oneapi/tbb/detail/_flow_graph_impl.h +++ b/include/oneapi/tbb/detail/_flow_graph_impl.h @@ -526,7 +526,8 @@ inline graph_task* prioritize_task(graph& g, graph_task& gt) { //! priority queue, and a new critical task is created to take and execute a work item with //! the highest known priority. The reference counting responsibility is transferred to //! the new task. - d1::task* critical_task = gt.my_allocator.new_object(g.my_priority_queue, gt.my_allocator); + d1::small_object_allocator allocator; + d1::task* critical_task = allocator.new_object(g.my_priority_queue, allocator); __TBB_ASSERT( critical_task, "bad_alloc?" ); g.my_priority_queue.push(>); using tbb::detail::d1::submit; diff --git a/src/tbb/small_object_pool.cpp b/src/tbb/small_object_pool.cpp index 28d11d011d..dbbf77cf5d 100644 --- a/src/tbb/small_object_pool.cpp +++ b/src/tbb/small_object_pool.cpp @@ -40,6 +40,7 @@ void* __TBB_EXPORTED_FUNC allocate(d1::small_object_pool*& allocator, std::size_ // TODO: optimize if the allocator contains a valid pool. auto tls = governor::get_thread_data(); auto pool = tls->my_small_object_pool; + __TBB_ASSERT(allocator == nullptr || pool == allocator, "An attempt was made to allocate using another thread's small memory pool"); return pool->allocate_impl(allocator, number_of_bytes); } diff --git a/test/tbb/test_function_node.cpp b/test/tbb/test_function_node.cpp index 999adac189..d71697672a 100644 --- a/test/tbb/test_function_node.cpp +++ b/test/tbb/test_function_node.cpp @@ -806,3 +806,25 @@ TEST_CASE("test function_node try_put_and_wait") { test_try_put_and_wait(); } #endif + +// It was an issue when the critical task wrapper was allocated using the small object pool +// of the task being wrapped. Since the original task creates under the aggregator, there is no +// guarantee that the thread that requested the task creating is the same as actually created the task +// Mismatch between memory pull caused internal assertion failure while deallocating the task +//! \brief \ref regression +TEST_CASE("test critical tasks memory pool correctness") { + using node_type = tbb::flow::function_node; + constexpr int num_iterations = 10000; + int num_calls = 0; + auto node_body = [&](int) { ++num_calls; }; + + tbb::flow::graph g; + node_type node(g, tbb::flow::serial, node_body, tbb::flow::node_priority_t{1}); + + for (int i = 0; i < num_iterations; ++i) { + node.try_put(i); + } + + g.wait_for_all(); + REQUIRE_MESSAGE(num_calls == num_iterations, "Incorrect number of body executions"); +} From a2eafb8f8428f3da1802c2633b94c58a78a69cf3 Mon Sep 17 00:00:00 2001 From: "Boyarinov, Konstantin" Date: Wed, 8 Jan 2025 09:04:11 -0800 Subject: [PATCH 2/3] Fix copyrights --- include/oneapi/tbb/detail/_flow_graph_impl.h | 2 +- src/tbb/small_object_pool.cpp | 2 +- test/tbb/test_function_node.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/oneapi/tbb/detail/_flow_graph_impl.h b/include/oneapi/tbb/detail/_flow_graph_impl.h index 75ed58c186..9606eae184 100644 --- a/include/oneapi/tbb/detail/_flow_graph_impl.h +++ b/include/oneapi/tbb/detail/_flow_graph_impl.h @@ -1,5 +1,5 @@ /* - Copyright (c) 2005-2024 Intel Corporation + Copyright (c) 2005-2025 Intel Corporation Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/src/tbb/small_object_pool.cpp b/src/tbb/small_object_pool.cpp index dbbf77cf5d..6134a4c92b 100644 --- a/src/tbb/small_object_pool.cpp +++ b/src/tbb/small_object_pool.cpp @@ -1,5 +1,5 @@ /* - Copyright (c) 2020-2021 Intel Corporation + Copyright (c) 2020-2025 Intel Corporation Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/test/tbb/test_function_node.cpp b/test/tbb/test_function_node.cpp index d71697672a..9df3fb662a 100644 --- a/test/tbb/test_function_node.cpp +++ b/test/tbb/test_function_node.cpp @@ -1,5 +1,5 @@ /* - Copyright (c) 2005-2024 Intel Corporation + Copyright (c) 2005-2025 Intel Corporation Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. From 70b4f4062e419036a150cf1f2377f0de5cb56658 Mon Sep 17 00:00:00 2001 From: Konstantin Boyarinov Date: Thu, 9 Jan 2025 18:18:06 +0200 Subject: [PATCH 3/3] Update src/tbb/small_object_pool.cpp Co-authored-by: Aleksei Fedotov --- src/tbb/small_object_pool.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tbb/small_object_pool.cpp b/src/tbb/small_object_pool.cpp index 6134a4c92b..e25c81a128 100644 --- a/src/tbb/small_object_pool.cpp +++ b/src/tbb/small_object_pool.cpp @@ -40,7 +40,8 @@ void* __TBB_EXPORTED_FUNC allocate(d1::small_object_pool*& allocator, std::size_ // TODO: optimize if the allocator contains a valid pool. auto tls = governor::get_thread_data(); auto pool = tls->my_small_object_pool; - __TBB_ASSERT(allocator == nullptr || pool == allocator, "An attempt was made to allocate using another thread's small memory pool"); + __TBB_ASSERT(allocator == nullptr || pool == allocator, + "An attempt was made to allocate using another thread's small memory pool"); return pool->allocate_impl(allocator, number_of_bytes); }