Fix exception bug in asssignment.

The hash and key equality functions were assigned before allocating new
buckets. If that allocation failed, then the existing elements would be
left in place - so if accessed after the exception they could be in the
wrong buckets or equivalent elements could be incorrectly grouped
together.

include/boost/unordered/detail/table.hpp

@@ -584,7 +584,6 @@ namespace boost { namespace unordered { namespace detail {
         {
             // Strong exception safety.
             set_hash_functions new_func_this(*this, x);
-            new_func_this.commit();
             mlf_ = x.mlf_;
             recalculate_max_load();
 
@@ -597,6 +596,7 @@ namespace boost { namespace unordered { namespace detail {
                 clear_buckets();
             }
 
+            new_func_this.commit();
             static_cast<table_impl*>(this)->assign_buckets(x);
         }
 
@@ -663,11 +663,13 @@ namespace boost { namespace unordered { namespace detail {
             }
             else {
                 set_hash_functions new_func_this(*this, x);
-                new_func_this.commit();
                 mlf_ = x.mlf_;
                 recalculate_max_load();
 
-                if (!size_ && !x.size_) return;
+                if (!size_ && !x.size_) {
+                    new_func_this.commit();
+                    return;
+                }
 
                 if (x.size_ >= max_load_) {
                     create_buckets(min_buckets_for_size(x.size_));
@@ -676,6 +678,7 @@ namespace boost { namespace unordered { namespace detail {
                     clear_buckets();
                 }
 
+                new_func_this.commit();
                 static_cast<table_impl*>(this)->move_assign_buckets(x);
             }
         }

test/Jamfile.v2

@@ -72,6 +72,7 @@ test-suite unordered-exception
         [ run exception/constructor_exception_tests.cpp framework ]
         [ run exception/copy_exception_tests.cpp framework ]
         [ run exception/assign_exception_tests.cpp framework ]
+        [ run exception/move_assign_exception_tests.cpp framework ]
         [ run exception/insert_exception_tests.cpp framework ]
         [ run exception/erase_exception_tests.cpp framework ]
         [ run exception/rehash_exception_tests.cpp framework ]

test/exception/assign_exception_tests.cpp

@@ -111,6 +111,12 @@ struct assign_test4 : assign_values<T>
     assign_test4() : assign_values<T>(10, 10, 1, 2) {}
 };
 
+template <class T>
+struct assign_test4a : assign_values<T>
+{
+    assign_test4a() : assign_values<T>(10, 100, 1, 2) {}
+};
+
 template <class T>
 struct assign_test5 : assign_values<T>
 {
@@ -136,7 +142,7 @@ struct equivalent_test1 : assign_base<T>
 
 EXCEPTION_TESTS(
     (self_assign_test1)(self_assign_test2)
-    (assign_test1)(assign_test2)(assign_test3)(assign_test4)(assign_test5)
+    (assign_test1)(assign_test2)(assign_test3)(assign_test4)(assign_test4a)(assign_test5)
     (equivalent_test1),
     CONTAINER_SEQ)
 RUN_TESTS()

test/exception/move_assign_exception_tests.cpp

@@ -0,0 +1,131 @@
+
+// Copyright 2006-2009 Daniel James.
+// Distributed under the Boost Software License, Version 1.0. (See accompanying
+// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
+
+#include <iostream>
+#include "./containers.hpp"
+#include "../helpers/random_values.hpp"
+#include "../helpers/invariants.hpp"
+
+#if defined(BOOST_MSVC)
+#pragma warning(disable:4512) // move_assignment operator could not be generated
+#endif
+
+test::seed_t initialize_seed(12847);
+
+template <class T>
+struct move_assign_base : public test::exception_base
+{
+    test::random_values<T> x_values, y_values;
+    T x,y;
+
+    typedef BOOST_DEDUCED_TYPENAME T::hasher hasher;
+    typedef BOOST_DEDUCED_TYPENAME T::key_equal key_equal;
+    typedef BOOST_DEDUCED_TYPENAME T::allocator_type allocator_type;
+
+    move_assign_base(int tag1, int tag2, float mlf1 = 1.0, float mlf2 = 1.0) :
+        x_values(),
+        y_values(),
+        x(0, hasher(tag1), key_equal(tag1), allocator_type(tag1)),
+        y(0, hasher(tag2), key_equal(tag2), allocator_type(tag2))
+    {
+        x.max_load_factor(mlf1);
+        y.max_load_factor(mlf2);
+    }
+
+    typedef T data_type;
+    T init() const { return T(x); }
+    void run(T& x1) const {
+        test::exceptions_enable disable_exceptions(false);
+        T y1 = y;
+        disable_exceptions.release();
+        x1 = boost::move(y1);
+    }
+    void check BOOST_PREVENT_MACRO_SUBSTITUTION(T const& x1) const
+    {
+        test::check_equivalent_keys(x1);
+
+        // If the container is empty at the point of the exception, the
+        // internal structure is hidden, this exposes it, at the cost of
+        // messing up the data.
+        if (x_values.size()) {
+            T& x2 = const_cast<T&>(x1);
+            x2.emplace(*x_values.begin());
+            test::check_equivalent_keys(x2);
+        }
+    }
+};
+
+template <class T>
+struct move_assign_values : move_assign_base<T>
+{
+    move_assign_values(unsigned int count1, unsigned int count2,
+            int tag1, int tag2, float mlf1 = 1.0, float mlf2 = 1.0) :
+        move_assign_base<T>(tag1, tag2, mlf1, mlf2)
+    {
+        this->x_values.fill(count1);
+        this->y_values.fill(count2);
+        this->x.insert(this->x_values.begin(), this->x_values.end());
+        this->y.insert(this->y_values.begin(), this->y_values.end());
+    }
+};
+
+template <class T>
+struct move_assign_test1 : move_assign_values<T>
+{
+    move_assign_test1() : move_assign_values<T>(0, 0, 0, 0) {}
+};
+
+template <class T>
+struct move_assign_test2 : move_assign_values<T>
+{
+    move_assign_test2() : move_assign_values<T>(60, 0, 0, 0) {}
+};
+
+template <class T>
+struct move_assign_test3 : move_assign_values<T>
+{
+    move_assign_test3() : move_assign_values<T>(0, 60, 0, 0) {}
+};
+
+template <class T>
+struct move_assign_test4 : move_assign_values<T>
+{
+    move_assign_test4() : move_assign_values<T>(10, 10, 1, 2) {}
+};
+
+template <class T>
+struct move_assign_test4a : move_assign_values<T>
+{
+    move_assign_test4a() : move_assign_values<T>(10, 100, 1, 2) {}
+};
+
+template <class T>
+struct move_assign_test5 : move_assign_values<T>
+{
+    move_assign_test5() : move_assign_values<T>(5, 60, 0, 0, 1.0f, 0.1f) {}
+};
+
+template <class T>
+struct equivalent_test1 : move_assign_base<T>
+{
+    equivalent_test1() :
+        move_assign_base<T>(0, 0)
+    {
+        test::random_values<T> x_values2(10);
+        this->x_values.insert(x_values2.begin(), x_values2.end());
+        this->x_values.insert(x_values2.begin(), x_values2.end());
+        test::random_values<T> y_values2(10);
+        this->y_values.insert(y_values2.begin(), y_values2.end());
+        this->y_values.insert(y_values2.begin(), y_values2.end());
+        this->x.insert(this->x_values.begin(), this->x_values.end());
+        this->y.insert(this->y_values.begin(), this->y_values.end());
+    }
+};
+
+EXCEPTION_TESTS(
+    (move_assign_test1)(move_assign_test2)(move_assign_test3)(move_assign_test4)(move_assign_test4a)(move_assign_test5)
+    (equivalent_test1),
+    CONTAINER_SEQ)
+RUN_TESTS()

test/helpers/exception_test.hpp

@@ -111,16 +111,28 @@ namespace test {
         exceptions_enable(exceptions_enable const&);
 
         bool old_value_;
+        bool released_;
     public:
         exceptions_enable(bool enable)
-            : old_value_(exceptions_enabled)
+            : old_value_(exceptions_enabled), released_(false)
         {
             exceptions_enabled = enable;
         }
 
         ~exceptions_enable()
         {
+            if (!released_) {
                 exceptions_enabled = old_value_;
+                released_ = true;
+            }
+        }
+
+        void release()
+        {
+            if (!released_) {
+                exceptions_enabled = old_value_;
+                released_ = true;
+            }
         }
     };