diff --git a/include/function2/function2.hpp b/include/function2/function2.hpp index ee61132..fd8ba9c 100644 --- a/include/function2/function2.hpp +++ b/include/function2/function2.hpp @@ -175,7 +175,7 @@ constexpr auto overload(T&&... callables) { namespace type_erasure { /// Store the allocator inside the box template -struct box : public Allocator { +struct box : Allocator { T value_; explicit box(T value, Allocator allocator) @@ -197,6 +197,7 @@ struct box : public Allocator { std::decay_t>::template rebind_alloc>; real_allocator allocator(*static_cast(me)); + me->~box(); allocator.deallocate(me, 1U); } }; @@ -534,17 +535,24 @@ class vtable> { assert(box && "The object must not be over aligned or null!"); if (!IsInplace) { - // Just reassign both pointers if we allocated on the heap + // Just swap both pointers if we allocated on the heap to->ptr_ = from->ptr_; + #ifndef _NDEBUG + // We don't need to null the pointer since we know that + // we don't own the data anymore through the vtable + // which is set to empty. from->ptr_ = nullptr; #endif + to_table->set_allocated(); + } - // The object isn't allocate on the heap + // The object is allocated inplace else { construct(std::true_type{}, std::move(*box), to_table, to, to_capacity); + box->~T(); } return; } @@ -658,9 +666,10 @@ class vtable> { /// Moves the object at the given position void move(vtable& to_table, data_accessor* from, std::size_t from_capacity, - data_accessor* to, std::size_t to_capacity) const - noexcept(HasStrongExceptGuarantee) { + data_accessor* to, + std::size_t to_capacity) noexcept(HasStrongExceptGuarantee) { cmd_(&to_table, opcode::op_move, from, from_capacity, to, to_capacity); + set_empty(); } /// Destroys the object at the given position @@ -806,7 +815,6 @@ class erasure : internal_capacity_holder { Property::is_strong_exception_guaranteed) { right.vtable_.move(vtable_, right.opaque_ptr(), right.capacity(), this->opaque_ptr(), capacity()); - right.vtable_.destroy(right.opaque_ptr(), right.capacity()); } constexpr erasure(erasure const& right) { @@ -843,7 +851,6 @@ class erasure : internal_capacity_holder { vtable_.weak_destroy(this->opaque_ptr(), capacity()); right.vtable_.move(vtable_, right.opaque_ptr(), right.capacity(), this->opaque_ptr(), capacity()); - right.vtable_.destroy(right.opaque_ptr(), right.capacity()); return *this; } diff --git a/test/functionality-test.cpp b/test/functionality-test.cpp index 9dc171f..c98ffb1 100644 --- a/test/functionality-test.cpp +++ b/test/functionality-test.cpp @@ -20,6 +20,7 @@ class DeallocatorChecker { std::default_delete>{}(ptr); }); } + std::size_t operator()() const { return checker_->get(); } @@ -87,14 +88,29 @@ TYPED_TEST(AllSingleMoveAssignConstructTests, AreEmptyAfterNullptrAssign) { TYPED_TEST(AllSingleMoveAssignConstructTests, AreFreeingResourcesOnDestruction) { - std::size_t deallocates = 0UL; + // Pre test + { + std::size_t deallocates = 0UL; + + { + DeallocatorChecker checker{deallocates}; + ASSERT_EQ(deallocates, 0UL); + } + ASSERT_EQ(deallocates, 1UL); + } + + // Real test { - typename TestFixture::template left_t left( - DeallocatorChecker{deallocates}); - EXPECT_EQ(deallocates, 0UL); + std::size_t deallocates = 0UL; + + { + typename TestFixture::template left_t left( + DeallocatorChecker{deallocates}); + EXPECT_EQ(deallocates, 0UL); + } + EXPECT_EQ(deallocates, 1UL); } - EXPECT_EQ(deallocates, 1UL); } TYPED_TEST(AllSingleMoveAssignConstructTests, AreConstructibleFromFunctors) { diff --git a/test/regressions.cpp b/test/regressions.cpp index e392f20..1eb98fb 100644 --- a/test/regressions.cpp +++ b/test/regressions.cpp @@ -8,14 +8,19 @@ #include "function2-test.hpp" +struct stateful_callable { + std::string test; + + void operator()() { + } +}; + /// Iterator dereference (nullptr) crash in Visual Studio /// -/// This was caused through the fact that std::allocator allocates -/// uninitialized storage but calls operator::delete on the given -/// object which caused a double destruction. +/// This was caused through an issue with the allocated pointer swap on move TEST(regression_tests, move_iterator_dereference_nullptr) { std::string test = "hey"; - fu2::function fn = [test = std::move(test)]{}; + fu2::function fn = stateful_callable{std::move(test)}; auto fn2 = std::move(fn); (void)fn2;