Working around the need to use a lock during thread creation

STEllAR-GROUP · Jul 16, 2023 · 322edf4 · 322edf4
1 parent 0ad30ef
commit 322edf4
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 22 deletions.
diff --git a/libs/core/execution/include/hpx/execution/detail/future_exec.hpp b/libs/core/execution/include/hpx/execution/detail/future_exec.hpp
@@ -153,8 +153,8 @@ namespace hpx::lcos::detail {
     struct post_policy_spawner
     {
         template <typename F>
-        threads::thread_id_ref_type operator()(
-            F&& f, hpx::threads::thread_description desc) const
+        void operator()(F&& f, hpx::threads::thread_description desc,
+            threads::thread_id_ref_type& id) const
         {
             threads::thread_init_data data(
                 threads::make_thread_function_nullary(HPX_FORWARD(F, f)),
@@ -163,7 +163,7 @@ namespace hpx::lcos::detail {
                 threads::thread_stacksize::default_,
                 threads::thread_schedule_state::pending);
 
-            return threads::register_thread(data);
+            threads::register_thread(data, id);
         }
     };
 
@@ -173,11 +173,11 @@ namespace hpx::lcos::detail {
         Executor exec;
 
         template <typename F>
-        threads::thread_id_ref_type operator()(
-            F&& f, hpx::threads::thread_description) const
+        void operator()(F&& f, hpx::threads::thread_description,
+            threads::thread_id_ref_type& id) const
         {
+            id = threads::invalid_thread_id;
             hpx::parallel::execution::post(exec, HPX_FORWARD(F, f));
-            return threads::invalid_thread_id;
         }
     };
 

diff --git a/libs/core/futures/include/hpx/futures/futures_factory.hpp b/libs/core/futures/include/hpx/futures/futures_factory.hpp
@@ -8,6 +8,7 @@
 
 #include <hpx/config.hpp>
 #include <hpx/assert.hpp>
+
 #include <hpx/allocator_support/allocator_deleter.hpp>
 #include <hpx/allocator_support/internal_allocator.hpp>
 #include <hpx/async_base/launch_policy.hpp>

diff --git a/libs/core/futures/include/hpx/futures/packaged_continuation.hpp b/libs/core/futures/include/hpx/futures/packaged_continuation.hpp
@@ -230,17 +230,11 @@ namespace hpx::lcos::detail {
 
             hpx::intrusive_ptr<continuation> this_(this);
             hpx::threads::thread_description desc(f_, "async");
-            auto thrd = spawner(
+            spawner(
                 [this_ = HPX_MOVE(this_), f = HPX_MOVE(f)]() mutable -> void {
                     this_->template run_impl<Unwrap>(HPX_MOVE(f));
                 },
-                desc);
-
-            std::lock_guard<mutex_type> l(mtx_);
-            if (!this->base_type::is_ready())
-            {
-                this->runs_child_ = HPX_MOVE(thrd);
-            }
+                desc, this->runs_child_);
         }
 
     public:
@@ -261,8 +255,8 @@ namespace hpx::lcos::detail {
                     if (this->is_ready())
                         return;    // nothing we can do
 
-                    // 26110: Caller failing to hold lock 'l'
 #if defined(HPX_MSVC)
+// 26110: Caller failing to hold lock 'l'
 #pragma warning(push)
 #pragma warning(disable : 26110)
 #endif

diff --git a/libs/core/futures/tests/unit/direct_scoped_execution.cpp b/libs/core/futures/tests/unit/direct_scoped_execution.cpp
@@ -5,10 +5,10 @@
 //  Distributed under the Boost Software License, Version 1.0. (See accompanying
 //  file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
 
-#include <hpx/init.hpp>
 #include <hpx/chrono.hpp>
 #include <hpx/execution.hpp>
 #include <hpx/future.hpp>
+#include <hpx/init.hpp>
 #include <hpx/modules/format.hpp>
 #include <hpx/modules/testing.hpp>
 

diff --git a/libs/core/threading_base/include/hpx/threading_base/register_thread.hpp b/libs/core/threading_base/include/hpx/threading_base/register_thread.hpp
@@ -97,6 +97,7 @@ namespace hpx::threads {
     ///
     /// \param data       [in] The data to use for creating the thread.
     /// \param pool       [in] The thread pool to use for launching the work.
+    /// \param id         [out] The id of the newly created thread (if applicable)
     /// \param ec         [in,out] This represents the error status on exit,
     ///                   if this is pre-initialized to \a hpx#throws the
     ///                   function will throw on error instead.
@@ -111,14 +112,21 @@ namespace hpx::threads {
     ///                   throw but returns the result code using the parameter
     ///                   \a ec. Otherwise it throws an instance
     ///                   of hpx#exception.
-    inline threads::thread_id_ref_type register_thread(
-        threads::thread_init_data& data, threads::thread_pool_base* pool,
+    inline void register_thread(threads::thread_init_data& data,
+        threads::thread_pool_base* pool, threads::thread_id_ref_type& id,
         error_code& ec = throws)
     {
         HPX_ASSERT(pool);
         data.run_now = true;
-        threads::thread_id_ref_type id = threads::invalid_thread_id;
         pool->create_thread(data, id, ec);
+    }
+
+    inline threads::thread_id_ref_type register_thread(
+        threads::thread_init_data& data, threads::thread_pool_base* pool,
+        error_code& ec = throws)
+    {
+        threads::thread_id_ref_type id = threads::invalid_thread_id;
+        register_thread(data, pool, id, ec);
         return id;
     }
 
@@ -128,6 +136,7 @@ namespace hpx::threads {
     ///        on an HPX thread.
     ///
     /// \param data       [in] The data to use for creating the thread.
+    /// \param id         [out] The id of the newly created thread (if applicable)
     /// \param ec         [in,out] This represents the error status on exit,
     ///                   if this is pre-initialized to \a hpx#throws the
     ///                   function will throw on error instead.
@@ -141,6 +150,12 @@ namespace hpx::threads {
     ///                   \a hpx#throws this function doesn't throw but returns
     ///                   the result code using the parameter \a ec. Otherwise
     ///                   it throws an instance of hpx#exception.
+    inline void register_thread(threads::thread_init_data& data,
+        threads::thread_id_ref_type& id, error_code& ec = throws)
+    {
+        register_thread(data, detail::get_self_or_default_pool(), id, ec);
+    }
+
     inline threads::thread_id_ref_type register_thread(
         threads::thread_init_data& data, error_code& ec = throws)
     {

diff --git a/libs/core/threading_base/include/hpx/threading_base/thread_data.hpp b/libs/core/threading_base/include/hpx/threading_base/thread_data.hpp
@@ -9,6 +9,8 @@
 #pragma once
 
 #include <hpx/config.hpp>
+#include <hpx/assert.hpp>
+
 #include <hpx/concurrency/spinlock_pool.hpp>
 #include <hpx/coroutines/coroutine.hpp>
 #include <hpx/coroutines/detail/combined_tagged_state.hpp>
@@ -18,7 +20,6 @@
 #include <hpx/functional/function.hpp>
 #include <hpx/modules/errors.hpp>
 #include <hpx/modules/logging.hpp>
-#include <hpx/modules/memory.hpp>
 #include <hpx/threading_base/thread_description.hpp>
 #include <hpx/threading_base/thread_init_data.hpp>
 #if defined(HPX_HAVE_APEX)
@@ -31,10 +32,12 @@
 #include <forward_list>
 #include <memory>
 #include <mutex>
-#include <stack>
-#include <string>
 #include <utility>
 
+#if defined(HPX_HAVE_THREAD_BACKTRACE_ON_SUSPENSION)
+#include <string>
+#endif
+
 #include <hpx/config/warnings_prefix.hpp>
 
 ////////////////////////////////////////////////////////////////////////////////