Merge #6321

6321: Ensure hpx_main is a proper thread_function r=hkaiser a=Pansysk75

Ensure that hpx_main is launched as a thread_function, so that exit callbacks are respected, and the following code will actually be used:

https://github.com/STEllAR-GROUP/hpx/blob/e7c31a49dd4e550a61134e498f3ecfb585d201cb/libs/core/threading_base/include/hpx/threading_base/register_thread.hpp#L28-L52

I should add that previously the callbacks were never executed, so using add_thread_exit_callback would cause an assertion failure later on thread destruction (L163), due to this sequence of calls:

https://github.com/STEllAR-GROUP/hpx/blob/e7c31a49dd4e550a61134e498f3ecfb585d201cb/libs/core/threading_base/src/thread_data.cpp#L107-L111

https://github.com/STEllAR-GROUP/hpx/blob/e7c31a49dd4e550a61134e498f3ecfb585d201cb/libs/core/threading_base/src/thread_data.cpp#L157-L166

`@hkaiser` Let me know if this is a good idea, and whether you'd like me to add a regression test.

Co-authored-by: Panos <pansysk75@gmail.com>
Co-authored-by: Hartmut Kaiser <hartmut.kaiser@gmail.com>

libs/core/runtime_local/src/runtime_local.cpp

@@ -1460,11 +1460,13 @@ namespace hpx {
         lbt_ << "(1st stage) runtime::start: launching run_helper "
                 "HPX thread";
 
-        threads::thread_init_data data(
-            hpx::bind(&runtime::run_helper, this, func, std::ref(result_), true,
-                &detail::handle_print_bind),
-            "run_helper", threads::thread_priority::normal,
-            threads::thread_schedule_hint(0), threads::thread_stacksize::large);
+        threads::thread_function_type thread_func =
+            threads::make_thread_function(hpx::bind(&runtime::run_helper, this,
+                func, std::ref(result_), true, &detail::handle_print_bind));
+
+        threads::thread_init_data data(HPX_MOVE(thread_func), "run_helper",
+            threads::thread_priority::normal, threads::thread_schedule_hint(0),
+            threads::thread_stacksize::large);
 
         this->runtime::starting();
         threads::thread_id_ref_type id = threads::invalid_thread_id;

libs/full/init_runtime/src/hpx_init.cpp

@@ -130,11 +130,17 @@ namespace hpx::detail {
         freebsd_environ = env;
 #endif
         // set a handler for std::abort, std::at_quick_exit, and std::atexit
-        std::signal(SIGABRT, detail::on_abort);
-        std::atexit(detail::on_exit);
+        [[maybe_unused]] auto const prev_signal =
+            std::signal(SIGABRT, detail::on_abort);
+        HPX_ASSERT(prev_signal != SIG_ERR);
+
+        [[maybe_unused]] auto const ret_at_exit = std::atexit(detail::on_exit);
+        HPX_ASSERT(ret_at_exit == 0);
+
 #if defined(HPX_HAVE_CXX11_STD_QUICK_EXIT)
-        [[maybe_unused]] int const ret = std::at_quick_exit(detail::on_exit);
-        HPX_ASSERT(ret == 0);
+        [[maybe_unused]] auto const ret_at_quick_exit =
+            std::at_quick_exit(detail::on_exit);
+        HPX_ASSERT(ret_at_quick_exit == 0);
 #endif
         return detail::run_or_start(f, argc, argv, params, true);
     }
@@ -163,11 +169,17 @@ namespace hpx::detail {
         freebsd_environ = env;
 #endif
         // set a handler for std::abort, std::at_quick_exit, and std::atexit
-        std::signal(SIGABRT, detail::on_abort);
-        std::atexit(detail::on_exit);
+        [[maybe_unused]] auto const prev_signal =
+            std::signal(SIGABRT, detail::on_abort);
+        HPX_ASSERT(prev_signal != SIG_ERR);
+
+        [[maybe_unused]] auto const ret_atexit = std::atexit(detail::on_exit);
+        HPX_ASSERT(ret_atexit == 0);
+
 #if defined(HPX_HAVE_CXX11_STD_QUICK_EXIT)
-        [[maybe_unused]] int const ret = std::at_quick_exit(detail::on_exit);
-        HPX_ASSERT(ret == 0);
+        [[maybe_unused]] auto const ret_at_quick_exit =
+            std::at_quick_exit(detail::on_exit);
+        HPX_ASSERT(ret_at_quick_exit == 0);
 #endif
         return 0 == detail::run_or_start(f, argc, argv, params, false);
     }
@@ -870,15 +882,15 @@ namespace hpx {
                     hpx_startup::user_main_config(params.cfg), f};
 #endif
 
+                std::vector<
+                    std::shared_ptr<components::component_registry_base>>
+                    component_registries;
+
                 // scope exception handling to resource partitioner initialization
                 // any exception thrown during run_or_start below are handled
                 // separately
                 try
                 {
-                    std::vector<
-                        std::shared_ptr<components::component_registry_base>>
-                        component_registries;
-
                     result = cmdline.call(
                         params.desc_cmdline, argc, argv, component_registries);
 
@@ -903,13 +915,6 @@ namespace hpx {
                         hpx::resource::detail::make_partitioner(
                             params.rp_mode, cmdline.rtcfg_, affinity_data);
 
-                    for (auto& registry : component_registries)
-                    {
-                        hpx::register_startup_function([registry]() {
-                            registry->register_component_type();
-                        });
-                    }
-
                     activate_global_options(cmdline, argc, argv);
 
                     // check whether HPX should be exited at this point
@@ -964,6 +969,13 @@ namespace hpx {
                 default:
                 {
 #if defined(HPX_HAVE_DISTRIBUTED_RUNTIME)
+                    for (auto const& registry : component_registries)
+                    {
+                        hpx::register_startup_function([registry]() {
+                            registry->register_component_type();
+                        });
+                    }
+
                     LPROGRESS_ << "creating distributed runtime";
                     rt.reset(new hpx::runtime_distributed(cmdline.rtcfg_,
                         &hpx::detail::pre_main, &hpx::detail::post_main));
@@ -1011,7 +1023,7 @@ namespace hpx {
                 }
                 catch (hpx::util::bad_lexical_cast const&)
                 {
-                    ;    // do nothing
+                    // do nothing
                 }
             }
             return default_;
@@ -1112,9 +1124,8 @@ namespace hpx {
         if (std::abs(shutdown_timeout + 1.0) < 1e-16)
             shutdown_timeout = detail::get_option("hpx.shutdown_timeout", -1.0);
 
-        components::server::runtime_support* p =
-            static_cast<components::server::runtime_support*>(
-                get_runtime_distributed().get_runtime_support_lva());
+        auto* p = static_cast<components::server::runtime_support*>(
+            get_runtime_distributed().get_runtime_support_lva());
 
         if (nullptr == p)
         {
@@ -1147,9 +1158,8 @@ namespace hpx {
         }
 
 #if defined(HPX_HAVE_DISTRIBUTED_RUNTIME)
-        components::server::runtime_support* p =
-            static_cast<components::server::runtime_support*>(
-                get_runtime_distributed().get_runtime_support_lva());
+        auto* p = static_cast<components::server::runtime_support*>(
+            get_runtime_distributed().get_runtime_support_lva());
 
         if (nullptr == p)
         {

libs/full/runtime_distributed/src/runtime_distributed.cpp

@@ -472,11 +472,14 @@ namespace hpx {
         lbt_ << "(1st stage) runtime_distributed::start: launching "
                 "run_helper HPX thread";
 
-        threads::thread_init_data data(
-            hpx::bind(&runtime_distributed::run_helper, this, func,
-                std::ref(result_)),
-            "run_helper", threads::thread_priority::normal,
-            threads::thread_schedule_hint(0), threads::thread_stacksize::large);
+        threads::thread_function_type thread_func =
+            threads::make_thread_function(
+                hpx::bind(&runtime_distributed::run_helper, this, func,
+                    std::ref(result_)));
+
+        threads::thread_init_data data(HPX_MOVE(thread_func), "run_helper",
+            threads::thread_priority::normal, threads::thread_schedule_hint(0),
+            threads::thread_stacksize::large);
 
         this->runtime::starting();
         threads::thread_id_ref_type id = threads::invalid_thread_id;

tests/regressions/threads/CMakeLists.txt

@@ -7,6 +7,7 @@
 
 set(tests
     block_os_threads_1036
+    main_thread_exit_callbacks
     run_as_hpx_thread_exceptions_3304
     run_as_os_thread_lockup_2991
     stackless_self_4155

tests/regressions/threads/main_thread_exit_callbacks.cpp

@@ -0,0 +1,55 @@
+//  Copyright (c) 2023 Panos Syskakis
+//
+//  SPDX-License-Identifier: BSL-1.0
+//  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/modules/testing.hpp>
+#include <hpx/runtime.hpp>
+#include <hpx/thread.hpp>
+
+#include <atomic>
+#include <cstddef>
+
+bool callback_called(false);
+
+int hpx_main()
+{
+    hpx::threads::thread_id_type id = hpx::threads::get_self_id();
+    hpx::threads::add_thread_exit_callback(id, [id]() {
+        hpx::threads::thread_id_type id1 = hpx::threads::get_self_id();
+        HPX_TEST_EQ(id1, id);
+
+        callback_called = true;
+    });
+
+    return hpx::finalize();
+}
+
+int main(int argc, char* argv[])
+{
+    // Test local runtime
+    {
+        hpx::init_params iparams;
+        iparams.mode = hpx::runtime_mode::local;
+        callback_called = false;
+        HPX_TEST_EQ_MSG(hpx::init(argc, argv, iparams), 0,
+            "HPX main exited with non-zero status");
+        HPX_TEST(callback_called);
+    }
+
+#if defined(HPX_HAVE_DISTRIBUTED_RUNTIME)
+    // Test distributed runtime
+    {
+        hpx::init_params iparams;
+        iparams.mode = hpx::runtime_mode::console;
+        callback_called = false;
+        HPX_TEST_EQ_MSG(hpx::init(argc, argv, iparams), 0,
+            "HPX main exited with non-zero status");
+
+        HPX_TEST(callback_called);
+    }
+#endif
+    return hpx::util::report_errors();
+}