Skip to content

Commit

Permalink
Switch from scope_guar to try-catch
Browse files Browse the repository at this point in the history
MSVC 2022 seems to miscompile `spawn_future`'s use of a `scope_guard`
(destructors are run out-of-order, violating invariants) so switch to a
try-catch block.
  • Loading branch information
ispeters committed Sep 1, 2024
1 parent 59389cf commit 1e59ff8
Showing 1 changed file with 28 additions and 27 deletions.
55 changes: 28 additions & 27 deletions include/unifex/spawn_future.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -954,38 +954,39 @@ struct _spawn_future_fn {
alloc_traits_t::construct(opAlloc, op, opAlloc);

// the next two steps might throw and we have to destroy op if one of them
// does throw so set up a scope_guard that will do that for us;
// conveniently, the deleter() static method does exactly the right thing
scope_guard cleanUp = [op]() noexcept {
// does throw; we used to use a scope_guard here but it triggers a
// miscompilation bug in VS 2022's MSVC that isn't triggered with a
// traditional try-catch block
UNIFEX_TRY {
using future_t = future_for<remove_cvref_t<Sender>, Scope, byte_alloc_t>;

// construct the future that we'll hand back to the caller; this is fairly
// likely to be noexcept since it's not much more than a Sender construction
// but it depends on the implementation of nest() for scope so it might
// throw
future_t future{scope, op};

// now finally construct the spawned operation, which might throw
op->init_operation(static_cast<Sender&&>(sender), scope, op);

// now that everything is wired together there are no more exception
// concerns from here

// start the spawned operation
start(*op);

// ideally, the compiler performs NRVO and constructs this future in-place
// at the call site but future<>'s move constructor is noexcept so even if
// there's an actual move here, there's still no chance of an exception
return future;
} UNIFEX_CATCH(...) {
// Constructing the future is *almost* no-throw--only the call to nest()
// might throw--so the future will invoke drop() on the operation (moving
// it from init to complete) as part of its destructor before this code
// runs.
op_t::deleter(op, _future_state::complete);
};

using future_t = future_for<remove_cvref_t<Sender>, Scope, byte_alloc_t>;

// construct the future that we'll hand back to the caller; this is fairly
// likely to be noexcept since it's not much more than a Sender construction
// but it depends on the implementation of nest() for scope so it might
// throw
future_t future{scope, op};

// now finally construct the spawned operation, which might throw
op->init_operation(static_cast<Sender&&>(sender), scope, op);

// now that everything is wired together there are no more exception
// concerns
cleanUp.release();

// start the spawned operation
start(*op);

// ideally, the compiler performs NRVO and constructs this future in-place
// at the call site but future<>'s move constructor is noexcept so even if
// there's an actual move here, there's still no chance of an exception
return future;
throw;
}
}

template <typename Scope, typename Alloc = std::allocator<std::byte>>
Expand Down

0 comments on commit 1e59ff8

Please sign in to comment.