From 249c6fd57d952e05079b04e7cc6adebb9d15dce6 Mon Sep 17 00:00:00 2001 From: Mingxin Wang Date: Mon, 7 Sep 2026 21:41:39 -0400 Subject: [PATCH 1/2] Reset the proxy when the destructor of the underlying pointer throws proxy::destroy() invoked destruction through an lvalue-qualified overload, so the meta_resetting_guard that invoke_impl applies to consuming overloads never ran. When the destructor of the underlying pointer threw, the metadata still pointed at the destroyed object, so has_value() stayed true and the next destruction ran on a dead object. Every path that discards a value reached this: reset(), operator=(nullptr), both branches of the copy assignment operator, the move assignment operator, operator=(P&&) and both emplace overloads. Destruction is a consuming operation like relocation, so give it the same shape. The destroy meta now uses an rvalue-qualified overload, which makes erased_context destroy the pointer through destroying_guard and makes invoke_impl clear the metadata on both the normal and the exceptional path. destroy_dispatch keeps only its tag role and its call operator becomes a no-op, because the destruction it used to perform is what the rvalue machinery already does. The added reset is dead on the non-throwing path and the optimizer removes it. At -O2 the disassembly of ~proxy, reset and the move assignment operator is unchanged for a facade whose destructibility is nothrow. LifetimeTracker gains ThrowingDestructionSession, a Session whose destructor throws, which is the first pointer in the suite with a potentially throwing destructor and the first use of a facade whose destructibility is nontrivial. --- include/proxy/v4/detail/core.h | 13 +++--- tests/proxy_lifetime_tests.cpp | 75 ++++++++++++++++++++++++++++++++++ tests/utils.h | 10 +++++ 3 files changed, 90 insertions(+), 8 deletions(-) diff --git a/include/proxy/v4/detail/core.h b/include/proxy/v4/detail/core.h index 4262904b..d19d243e 100644 --- a/include/proxy/v4/detail/core.h +++ b/include/proxy/v4/detail/core.h @@ -617,10 +617,7 @@ struct copy_dispatch { } }; struct destroy_dispatch { - template - PRO4D_STATIC_CALL(void, T& self) noexcept(std::is_nothrow_destructible_v) { - std::destroy_at(&self); - } + PRO4D_STATIC_CALL(void, auto&&) noexcept {} }; template struct lifetime_meta_traits : std::type_identity {}; @@ -925,8 +922,8 @@ struct facade_traits : specialization_t, lifetime_meta_t, - lifetime_meta_t, + lifetime_meta_t, typename facade_traits::conv_meta, typename facade_traits::refl_meta>>; using indirect_accessor = composite_t< @@ -1331,8 +1328,8 @@ class proxy : public detail::facade_traits::direct_accessor, if constexpr (F::destructibility != constraint_level::trivial) { if (meta_.has_value()) { invoke(*this); + void() && noexcept(F::destructibility == + constraint_level::nothrow)>(std::move(*this)); } } } diff --git a/tests/proxy_lifetime_tests.cpp b/tests/proxy_lifetime_tests.cpp index 701e2ce4..ccb56963 100644 --- a/tests/proxy_lifetime_tests.cpp +++ b/tests/proxy_lifetime_tests.cpp @@ -26,6 +26,14 @@ struct TestTrivialFacade ::support_destruction // ::build {}; +struct TestThrowingDestructionFacade + : pro::facade_builder // + ::add_convention // + ::support_copy // + ::support_relocation // + ::support_destruction // + ::build {}; + struct TestRttiFacade : pro::facade_builder // ::add_direct_reflection // ::add_facade_with_substitution // @@ -274,6 +282,21 @@ TEST(ProxyLifetimeTests, TestMoveConstrction_FromNull) { ASSERT_FALSE(p2.has_value()); } +TEST(ProxyLifetimeTests, TestDestruction_Exception) { + utils::LifetimeTracker tracker; + std::vector expected_ops; + auto destroy = [&] { + pro::proxy p{ + std::in_place_type, + &tracker}; + }; + ASSERT_THROW(destroy(), utils::DestructionFailure); + expected_ops.emplace_back(1, + utils::LifetimeOperationType::kValueConstruction); + expected_ops.emplace_back(1, utils::LifetimeOperationType::kDestruction); + ASSERT_TRUE(tracker.GetOperations() == expected_ops); +} + TEST(ProxyLifetimeTests, TestNullAssignment_FromNullptr_ToValue) { utils::LifetimeTracker tracker; std::vector expected_ops; @@ -358,6 +381,30 @@ TEST(ProxyLifetimeTests, TestPolyAssignment_ToValue_Exception) { ASSERT_TRUE(tracker.GetOperations() == expected_ops); } +TEST(ProxyLifetimeTests, TestPolyAssignment_ToValue_DestructionException) { + utils::LifetimeTracker tracker; + std::vector expected_ops; + { + pro::proxy p{ + std::in_place_type, + &tracker}; + expected_ops.emplace_back(1, + utils::LifetimeOperationType::kValueConstruction); + utils::LifetimeTracker::Session session{&tracker}; + expected_ops.emplace_back(2, + utils::LifetimeOperationType::kValueConstruction); + ASSERT_THROW(p = session, utils::DestructionFailure); + ASSERT_FALSE(p.has_value()); + expected_ops.emplace_back(3, + utils::LifetimeOperationType::kCopyConstruction); + expected_ops.emplace_back(1, utils::LifetimeOperationType::kDestruction); + expected_ops.emplace_back(3, utils::LifetimeOperationType::kDestruction); + ASSERT_TRUE(tracker.GetOperations() == expected_ops); + } + expected_ops.emplace_back(2, utils::LifetimeOperationType::kDestruction); + ASSERT_TRUE(tracker.GetOperations() == expected_ops); +} + TEST(ProxyLifetimeTests, TestPolyAssignment_FromValue_ToNull) { utils::LifetimeTracker tracker; std::vector expected_ops; @@ -630,6 +677,34 @@ TEST(ProxyLifetimeTests, TestCopyAssignment_FromValue_ToValue_Exception) { ASSERT_TRUE(tracker.GetOperations() == expected_ops); } +TEST(ProxyLifetimeTests, + TestCopyAssignment_FromValue_ToValue_DestructionException) { + utils::LifetimeTracker tracker; + std::vector expected_ops; + { + pro::proxy p1{ + std::in_place_type, + &tracker}; + expected_ops.emplace_back(1, + utils::LifetimeOperationType::kValueConstruction); + pro::proxy p2{ + std::in_place_type, &tracker}; + expected_ops.emplace_back(2, + utils::LifetimeOperationType::kValueConstruction); + ASSERT_THROW(p1 = p2, utils::DestructionFailure); + ASSERT_FALSE(p1.has_value()); + ASSERT_TRUE(p2.has_value()); + ASSERT_EQ(ToString(*p2), "Session 2"); + expected_ops.emplace_back(3, + utils::LifetimeOperationType::kCopyConstruction); + expected_ops.emplace_back(1, utils::LifetimeOperationType::kDestruction); + expected_ops.emplace_back(3, utils::LifetimeOperationType::kDestruction); + ASSERT_TRUE(tracker.GetOperations() == expected_ops); + } + expected_ops.emplace_back(2, utils::LifetimeOperationType::kDestruction); + ASSERT_TRUE(tracker.GetOperations() == expected_ops); +} + TEST(ProxyLifetimeTests, TestCopyAssignment_FromValue_ToSelf) { utils::LifetimeTracker tracker; std::vector expected_ops; diff --git a/tests/utils.h b/tests/utils.h index ab53e639..e58ee14e 100644 --- a/tests/utils.h +++ b/tests/utils.h @@ -38,6 +38,8 @@ struct ConstructionFailure : std::exception { LifetimeOperationType type_; }; +struct DestructionFailure : std::exception {}; + class LifetimeTracker { public: LifetimeTracker() = default; @@ -74,6 +76,14 @@ class LifetimeTracker { LifetimeTracker* const host_; }; + class ThrowingDestructionSession : public Session { + public: + using Session::Session; + ~ThrowingDestructionSession() noexcept(false) { + throw DestructionFailure{}; + } + }; + const std::vector& GetOperations() const { return ops_; } void ThrowOnNextConstruction() { throw_on_next_construction_ = true; } From 7ec78d0a4b3d549fa1690f05a4e7203453f3365b Mon Sep 17 00:00:00 2001 From: Mingxin Wang Date: Wed, 9 Sep 2026 10:00:21 +0800 Subject: [PATCH 2/2] Fix MSVC failure --- include/proxy/v4/detail/core.h | 4 +--- include/proxy/v4/detail/proxy_creation.h | 4 ++-- tests/utils.h | 5 ++++- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/include/proxy/v4/detail/core.h b/include/proxy/v4/detail/core.h index d19d243e..39642bc4 100644 --- a/include/proxy/v4/detail/core.h +++ b/include/proxy/v4/detail/core.h @@ -385,9 +385,7 @@ R invoke_dispatch(Args&&... args) { template struct destroying_guard { explicit destroying_guard(P* p) noexcept : p_(p) {} - ~destroying_guard() noexcept(std::is_nothrow_destructible_v

) { - std::destroy_at(p_); - } + ~destroying_guard() noexcept(std::is_nothrow_destructible_v

) { p_->~P(); } private: P* p_; diff --git a/include/proxy/v4/detail/proxy_creation.h b/include/proxy/v4/detail/proxy_creation.h index 2c4cf522..af7cc082 100644 --- a/include/proxy/v4/detail/proxy_creation.h +++ b/include/proxy/v4/detail/proxy_creation.h @@ -78,7 +78,7 @@ template void deallocate(const Alloc& alloc, T* ptr) { auto al = typename std::allocator_traits::template rebind_alloc(alloc); - std::destroy_at(ptr); + ptr->~T(); al.deallocate(ptr, 1); } template @@ -223,7 +223,7 @@ class strong_compact_ptr { strong_compact_ptr(strong_compact_ptr&& rhs) = delete; ~strong_compact_ptr() noexcept(std::is_nothrow_destructible_v) { if (ptr_->strong_count.fetch_sub(1, std::memory_order::acq_rel) == 1) { - std::destroy_at(operator->()); + operator->()->~T(); if (ptr_->weak_count.fetch_sub(1u, std::memory_order::release) == 1) { deallocate(ptr_->alloc, ptr_); } diff --git a/tests/utils.h b/tests/utils.h index e58ee14e..e728aec0 100644 --- a/tests/utils.h +++ b/tests/utils.h @@ -5,6 +5,7 @@ #ifndef _MSFT_PROXY_TEST_UTILS_ #define _MSFT_PROXY_TEST_UTILS_ +#include #include #include #include @@ -80,7 +81,9 @@ class LifetimeTracker { public: using Session::Session; ~ThrowingDestructionSession() noexcept(false) { - throw DestructionFailure{}; + if (std::uncaught_exceptions() == 0) { + throw DestructionFailure{}; + } } };