Skip to content

Commit

Permalink
Merge #6278 #6279 #6281
Browse files Browse the repository at this point in the history
6278: Making sure the future's shared state doesn't go out of scope prematurely r=hkaiser a=hkaiser



6279: Re-expose error names r=hkaiser a=hkaiser

In one of the recent HPX versions, the previously publicly available string representations of the HPX error code were removed from the public API. This PR introduces an API that allows to query the error names.

6281: Creating directory for file copy r=hkaiser a=hkaiser



Co-authored-by: Hartmut Kaiser <hartmut.kaiser@gmail.com>
  • Loading branch information
StellarBot and hkaiser committed Jun 17, 2023
4 parents b75ec6f + e4ea6d6 + 213fe3a + fef1c92 commit f1be2d0
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 38 deletions.
5 changes: 3 additions & 2 deletions cmake/HPX_AddModule.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,9 @@ function(add_hpx_module libname modulename)
set(generated_file_base "${CMAKE_CURRENT_BINARY_DIR}/include")
if(NOT HPX_WITH_DISTRIBUTED_RUNTIME)
foreach(file_to_generate ${${modulename}_GENERATED_HEADERS})
file(COPY_FILE include/${file_to_generate}.in
${generated_file_base}/${file_to_generate} ONLY_IF_DIFFERENT
configure_file(
${HEADER_ROOT}/${file_to_generate}.in
${generated_file_base}/${file_to_generate} COPYONLY
)
set(generated_headers ${generated_headers}
${generated_file_base}/${file_to_generate}
Expand Down
3 changes: 3 additions & 0 deletions libs/core/errors/include/hpx/errors/error.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,9 @@ namespace hpx {

#undef HPX_ERROR_UNSCOPED_ENUM_DEPRECATION_MSG

// Return a textual representation of a given error code
HPX_CORE_EXPORT char const* get_error_name(error e) noexcept;

} // namespace hpx

/// \cond NOEXTERNAL
Expand Down
16 changes: 15 additions & 1 deletion libs/core/errors/src/error_code.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,20 @@ namespace hpx {
/* */ ""};
/// \endcond

// Return a textual representation of a given error code
char const* get_error_name(error value) noexcept
{
if (value >= hpx::error::success && value < hpx::error::last_error)
{
return error_names[static_cast<int>(value)];
}
if (value & hpx::error::system_error_flag)
{
return "system_error";
}
return "unknown";
}

namespace detail {

class hpx_category : public std::error_category
Expand All @@ -99,7 +113,7 @@ namespace hpx {
}
if (value & hpx::error::system_error_flag)
{
return std::string("HPX(system_error)");
return "HPX(system_error)";
}
return "HPX(unknown_error)";
}
Expand Down
63 changes: 39 additions & 24 deletions libs/core/futures/include/hpx/futures/detail/future_data.hpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2007-2022 Hartmut Kaiser
// Copyright (c) 2007-2023 Hartmut Kaiser
//
// SPDX-License-Identifier: BSL-1.0
// Distributed under the Boost Software License, Version 1.0. (See accompanying
Expand All @@ -9,7 +9,6 @@
#include <hpx/config.hpp>
#include <hpx/assert.hpp>
#include <hpx/async_base/launch_policy.hpp>
#include <hpx/coroutines/detail/get_stack_pointer.hpp>
#include <hpx/datastructures/detail/small_vector.hpp>
#include <hpx/errors/try_catch_exception_ptr.hpp>
#include <hpx/functional/function.hpp>
Expand All @@ -22,7 +21,6 @@
#include <hpx/synchronization/spinlock.hpp>
#include <hpx/thread_support/assert_owns_lock.hpp>
#include <hpx/thread_support/atomic_count.hpp>
#include <hpx/threading_base/annotated_function.hpp>
#include <hpx/threading_base/thread_helpers.hpp>
#include <hpx/type_support/construct_at.hpp>
#include <hpx/type_support/unused.hpp>
Expand All @@ -33,7 +31,6 @@
#include <exception>
#include <memory>
#include <mutex>
#include <string>
#include <type_traits>
#include <utility>

Expand Down Expand Up @@ -242,7 +239,12 @@ namespace hpx::lcos::detail {
using result_type = util::unused_type;
using init_no_addref = future_data_refcnt_base::init_no_addref;

virtual ~future_data_base();
future_data_base(future_data_base const&) = delete;
future_data_base(future_data_base&&) = delete;
future_data_base& operator=(future_data_base const&) = delete;
future_data_base& operator=(future_data_base&&) = delete;

~future_data_base() override;

enum state
{
Expand Down Expand Up @@ -319,6 +321,7 @@ namespace hpx::lcos::detail {
protected:
mutable mutex_type mtx_;
std::atomic<state> state_; // current state

completed_callback_vector_type on_completed_;
local::detail::condition_variable cond_; // threads waiting in read
};
Expand Down Expand Up @@ -375,7 +378,7 @@ namespace hpx::lcos::detail {
future_data_base(init_no_addref no_addref, std::in_place_t, Ts&&... ts)
: base_type(no_addref)
{
result_type* value_ptr = reinterpret_cast<result_type*>(&storage_);
auto* value_ptr = reinterpret_cast<result_type*>(&storage_);
construct(value_ptr, HPX_FORWARD(Ts, ts)...);
state_.store(value, std::memory_order_relaxed);
}
Expand All @@ -384,12 +387,17 @@ namespace hpx::lcos::detail {
init_no_addref no_addref, std::exception_ptr e) noexcept
: base_type(no_addref)
{
std::exception_ptr* exception_ptr =
auto* exception_ptr =
reinterpret_cast<std::exception_ptr*>(&storage_);
hpx::construct_at(exception_ptr, HPX_MOVE(e));
state_.store(exception, std::memory_order_relaxed);
}

future_data_base(future_data_base const&) = delete;
future_data_base(future_data_base&&) = delete;
future_data_base& operator=(future_data_base const&) = delete;
future_data_base& operator=(future_data_base&&) = delete;

~future_data_base() noexcept override
{
reset();
Expand Down Expand Up @@ -430,14 +438,16 @@ namespace hpx::lcos::detail {
template <typename... Ts>
void set_value(Ts&&... ts)
{
hpx::intrusive_ptr<future_data_base> this_(this); // keep alive

// Note: it is safe to access the data store as no other thread
// should access it concurrently. There shouldn't be any
// threads attempting to read the value as the state is still
// empty. Also, there can be only one thread (this thread)
// attempting to set the value by definition.

// set the data
result_type* value_ptr = reinterpret_cast<result_type*>(&storage_);
auto* value_ptr = reinterpret_cast<result_type*>(&storage_);
construct(value_ptr, HPX_FORWARD(Ts, ts)...);

// At this point the lock needs to be acquired to safely access the
Expand All @@ -460,7 +470,6 @@ namespace hpx::lcos::detail {
HPX_THROW_EXCEPTION(hpx::error::promise_already_satisfied,
"future_data_base::set_value",
"data has already been set for this future");
return;
}

// 26111: Caller failing to release lock 'this->mtx_'
Expand Down Expand Up @@ -501,14 +510,16 @@ namespace hpx::lcos::detail {

void set_exception(std::exception_ptr data) override
{
hpx::intrusive_ptr<future_data_base> this_(this); // keep alive

// Note: it is safe to access the data store as no other thread
// should access it concurrently. There shouldn't be any
// threads attempting to read the value as the state is still
// empty. Also, there can be only one thread (this thread)
// attempting to set the value by definition.

// set the data
std::exception_ptr* exception_ptr =
auto* exception_ptr =
reinterpret_cast<std::exception_ptr*>(&storage_);
hpx::construct_at(exception_ptr, HPX_MOVE(data));

Expand All @@ -532,7 +543,6 @@ namespace hpx::lcos::detail {
HPX_THROW_EXCEPTION(hpx::error::promise_already_satisfied,
"future_data_base::set_exception",
"data has already been set for this future");
return;
}

// 26111: Caller failing to release lock 'this->mtx_'
Expand Down Expand Up @@ -620,14 +630,13 @@ namespace hpx::lcos::detail {
{
case value:
{
result_type* value_ptr =
reinterpret_cast<result_type*>(&storage_);
auto* value_ptr = reinterpret_cast<result_type*>(&storage_);
std::destroy_at(value_ptr);
break;
}
case exception:
{
std::exception_ptr* exception_ptr =
auto* exception_ptr =
reinterpret_cast<std::exception_ptr*>(&storage_);
std::destroy_at(exception_ptr);
break;
Expand Down Expand Up @@ -684,6 +693,11 @@ namespace hpx::lcos::detail {
{
}

future_data(future_data const&) = delete;
future_data(future_data&&) = delete;
future_data& operator=(future_data const&) = delete;
future_data& operator=(future_data&&) = delete;

~future_data() noexcept override = default;
};

Expand Down Expand Up @@ -775,7 +789,8 @@ namespace hpx::lcos::detail {
threads::thread_schedule_hint(),
threads::thread_stacksize::current,
threads::thread_schedule_state::suspended, true);
threads::thread_id_ref_type id = threads::register_thread(data, ec);
threads::thread_id_ref_type const id =
threads::register_thread(data, ec);
if (ec)
{
// thread creation failed, report error to the new future
Expand Down Expand Up @@ -893,7 +908,6 @@ namespace hpx::lcos::detail {
HPX_THROW_EXCEPTION(hpx::error::task_already_started,
"task_base::check_started",
"this task has already been started");
return;
}
started_ = true;
}
Expand Down Expand Up @@ -1010,6 +1024,9 @@ namespace hpx::lcos::detail {

void cancel() override
{
hpx::intrusive_ptr<cancelable_task_base> this_(
this); // keep alive

std::unique_lock<mutex_type> l(mtx_);
hpx::detail::try_catch_exception_ptr(
[&]() {
Expand Down Expand Up @@ -1056,13 +1073,11 @@ namespace hpx::lcos::detail {
};
} // namespace hpx::lcos::detail

namespace hpx::traits::detail {

template <typename R, typename Allocator>
struct shared_state_allocator<lcos::detail::future_data<R>, Allocator>
{
using type = lcos::detail::future_data_allocator<R, Allocator>;
};
} // namespace hpx::traits::detail
template <typename R, typename Allocator>
struct hpx::traits::detail::shared_state_allocator<
hpx::lcos::detail::future_data<R>, Allocator>
{
using type = hpx::lcos::detail::future_data_allocator<R, Allocator>;
};

#include <hpx/config/warnings_suffix.hpp>
37 changes: 26 additions & 11 deletions libs/core/futures/src/future_data.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2015-2022 Hartmut Kaiser
// Copyright (c) 2015-2023 Hartmut Kaiser
//
// SPDX-License-Identifier: BSL-1.0
// Distributed under the Boost Software License, Version 1.0. (See accompanying
Expand All @@ -16,7 +16,6 @@
#include <hpx/futures/futures_factory.hpp>
#include <hpx/modules/errors.hpp>
#include <hpx/modules/memory.hpp>
#include <hpx/threading_base/annotated_function.hpp>

#include <cstddef>
#include <exception>
Expand Down Expand Up @@ -44,6 +43,16 @@ namespace hpx::lcos::detail {
{
++count_;
}

handle_continuation_recursion_count(
handle_continuation_recursion_count const&) = delete;
handle_continuation_recursion_count(
handle_continuation_recursion_count&&) = delete;
handle_continuation_recursion_count& operator=(
handle_continuation_recursion_count const&) = delete;
handle_continuation_recursion_count& operator=(
handle_continuation_recursion_count&&) = delete;

~handle_continuation_recursion_count()
{
--count_;
Expand All @@ -58,7 +67,7 @@ namespace hpx::lcos::detail {
{
lcos::local::futures_factory<void()> p(HPX_FORWARD(Callback, f));

bool is_hpx_thread = nullptr != hpx::threads::get_self_ptr();
bool const is_hpx_thread = nullptr != hpx::threads::get_self_ptr();
hpx::launch policy = launch::fork;
if (!is_hpx_thread)
{
Expand All @@ -69,7 +78,7 @@ namespace hpx::lcos::detail {
policy.set_stacksize(threads::thread_stacksize::current);

// launch a new thread executing the given function
threads::thread_id_ref_type tid = //-V821
threads::thread_id_ref_type const tid = //-V821
p.post("run_on_completed_on_new_thread", policy);

// wait for the task to run
Expand Down Expand Up @@ -135,7 +144,7 @@ namespace hpx::lcos::detail {
// and promise::set_exception).
if (s == exception)
{
std::exception_ptr const* exception_ptr =
auto const* exception_ptr =
static_cast<std::exception_ptr const*>(storage);

// an error has been reported in the meantime, throw or set
Expand All @@ -145,10 +154,8 @@ namespace hpx::lcos::detail {
std::rethrow_exception(*exception_ptr);
// never reached
}
else
{
ec = make_error_code(*exception_ptr);
}

ec = make_error_code(*exception_ptr);
}

return nullptr;
Expand Down Expand Up @@ -199,7 +206,7 @@ namespace hpx::lcos::detail {
bool recurse_asynchronously =
!this_thread::has_sufficient_stack_space();
#else
handle_continuation_recursion_count cnt;
handle_continuation_recursion_count const cnt;
bool recurse_asynchronously =
cnt.count_ > HPX_CONTINUATION_MAX_RECURSION_DEPTH ||
(hpx::threads::get_self_ptr() == nullptr);
Expand All @@ -215,8 +222,10 @@ namespace hpx::lcos::detail {

hpx::detail::try_catch_exception_ptr(
[&]() {
constexpr void (*p)(Callback &&) noexcept =
// clang-format off
constexpr void (*p)(Callback&&) noexcept =
&future_data_base::run_on_completed;
// clang-format on
run_on_completed_on_new_thread(util::deferred_call(
p, HPX_FORWARD(Callback, on_completed)));
},
Expand Down Expand Up @@ -260,6 +269,8 @@ namespace hpx::lcos::detail {
}
else
{
hpx::intrusive_ptr<future_data_base> this_(this); // keep alive

std::unique_lock l(mtx_);
if (is_ready())
{
Expand All @@ -282,6 +293,8 @@ namespace hpx::lcos::detail {
state s = state_.load(std::memory_order_acquire);
if (s == empty)
{
hpx::intrusive_ptr<future_data_base> this_(this); // keep alive

std::unique_lock l(mtx_);
s = state_.load(std::memory_order_relaxed);
if (s == empty)
Expand Down Expand Up @@ -311,6 +324,8 @@ namespace hpx::lcos::detail {
// block if this entry is empty
if (state_.load(std::memory_order_acquire) == empty)
{
hpx::intrusive_ptr<future_data_base> this_(this); // keep alive

std::unique_lock l(mtx_);
if (state_.load(std::memory_order_relaxed) == empty)
{
Expand Down

0 comments on commit f1be2d0

Please sign in to comment.