Correct a mistake made in 5e37606

* The double free was caused through an issue
  in the pointer swap on allocated move.

include/function2/function2.hpp

@@ -175,7 +175,7 @@ constexpr auto overload(T&&... callables) {
 namespace type_erasure {
 /// Store the allocator inside the box
 template <typename T, typename Allocator>
-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<Allocator>>::template rebind_alloc<box<T, Allocator>>;
     real_allocator allocator(*static_cast<Allocator const*>(me));
 
+    me->~box();
     allocator.deallocate(me, 1U);
   }
 };
@@ -534,17 +535,24 @@ class vtable<property<IsThrowing, HasStrongExceptGuarantee, FormalArgs...>> {
           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<T>();
+
           }
-          // 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<property<IsThrowing, HasStrongExceptGuarantee, FormalArgs...>> {
 
   /// 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<Config::capacity> {
       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<Config::capacity> {
     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;
   }
 

test/functionality-test.cpp

@@ -20,6 +20,7 @@ class DeallocatorChecker {
           std::default_delete<std::reference_wrapper<std::size_t>>{}(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<std::size_t() const> left(
-        DeallocatorChecker{deallocates});
-    EXPECT_EQ(deallocates, 0UL);
+    std::size_t deallocates = 0UL;
+
+    {
+      typename TestFixture::template left_t<std::size_t() const> left(
+          DeallocatorChecker{deallocates});
+      EXPECT_EQ(deallocates, 0UL);
+    }
+    EXPECT_EQ(deallocates, 1UL);
   }
-  EXPECT_EQ(deallocates, 1UL);
 }
 
 TYPED_TEST(AllSingleMoveAssignConstructTests, AreConstructibleFromFunctors) {

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<void()> fn = [test = std::move(test)]{};
+  fu2::function<void()> fn = stateful_callable{std::move(test)};
 
   auto fn2 = std::move(fn);
   (void)fn2;