* Fixed serious bug in MichaelSet::emplace() function

New node was created twice from the arguments by move semantics. However, move semantics may change internal state of the argument. This can lead to an incorrect element in the set and even to an incorrect key that breaks the set logic.

diff --git a/cds/container/michael_list_nogc.h b/cds/container/michael_list_nogc.h
index c6bd1fd1d74252bc11b2df78951210d9b10d432a..b7fbbf6e6e980c2627d5a7d381a20025099b5a34 100644 (file)
--- a/cds/container/michael_list_nogc.h
+++ b/cds/container/michael_list_nogc.h
@@ -426,6 +426,16 @@ namespace cds { namespace container {
 
     protected:
         //@cond
+        static value_type& node_to_value( node_type& n )
+        {
+            return n.m_Value;
+        }
+
+        iterator insert_node( node_type * pNode )
+        {
+            return node_to_iterator( insert_node_at( head(), pNode ));
+        }
+
         node_type * insert_node_at( head_type& refHead, node_type * pNode )
         {
             assert( pNode != nullptr );

diff --git a/cds/container/michael_list_rcu.h b/cds/container/michael_list_rcu.h
index d4510a4e59d5f4b1a5728e0d77c41d437a71ca2c..3a9ba738e1cfd567c52f6958ab22065af832fbc3 100644 (file)
--- a/cds/container/michael_list_rcu.h
+++ b/cds/container/michael_list_rcu.h
@@ -191,7 +191,7 @@ namespace cds { namespace container {
         /// Result of \p get(), \p get_with() functions - pointer to the node found
         typedef cds::urcu::raw_ptr_adaptor< value_type, typename base_class::raw_ptr, raw_ptr_converter > raw_ptr;
 
-    private:
+    protected:
         //@cond
         static value_type& node_to_value( node_type& n )
         {
@@ -201,10 +201,7 @@ namespace cds { namespace container {
         {
             return n.m_Value;
         }
-        //@endcond
 
-    protected:
-        //@cond
         template <typename Q>
         static node_type * alloc_node( Q const& v )
         {
@@ -788,6 +785,11 @@ namespace cds { namespace container {
 
     protected:
         //@cond
+        bool insert_node( node_type * pNode )
+        {
+            return insert_node_at( head(), pNode );
+        }
+
         bool insert_node_at( head_type& refHead, node_type * pNode )
         {
             assert( pNode );

diff --git a/cds/container/michael_set_nogc.h b/cds/container/michael_set_nogc.h
index c4ae09b84f5011eeab5b4b161607dd22657d4518..b3e7db4be2203531c6ce1da68e597f187ec5f5ca 100644 (file)
--- a/cds/container/michael_set_nogc.h
+++ b/cds/container/michael_set_nogc.h
@@ -70,19 +70,31 @@ namespace cds { namespace container {
         typedef typename cds::opt::v::hash_selector< typename traits::hash >::type hash;
         typedef typename traits::item_counter  item_counter; ///< Item counter type
 
-        /// Bucket table allocator
-        typedef cds::details::Allocator< bucket_type, typename traits::allocator >  bucket_table_allocator;
-
     protected:
         //@cond
+        class internal_bucket_type: public bucket_type
+        {
+            typedef bucket_type base_class;
+        public:
+            using base_class::node_type;
+            using base_class::alloc_node;
+            using base_class::insert_node;
+            using base_class::node_to_value;
+        };
+
+        /// Bucket table allocator
+        typedef cds::details::Allocator< internal_bucket_type, typename traits::allocator >  bucket_table_allocator;
+
         typedef typename bucket_type::iterator        bucket_iterator;
         typedef typename bucket_type::const_iterator  bucket_const_iterator;
         //@endcond
 
     protected:
+        //@cond
         item_counter    m_ItemCounter;   ///< Item counter
         hash            m_HashFunctor;   ///< Hash functor
-        bucket_type *   m_Buckets;       ///< bucket table
+        internal_bucket_type *   m_Buckets;       ///< bucket table
+        //@endcond
 
     private:
         //@cond
@@ -100,7 +112,7 @@ namespace cds { namespace container {
 
         /// Returns the bucket (ordered list) for \p key
         template <typename Q>
-        bucket_type&    bucket( const Q& key )
+        internal_bucket_type& bucket( const Q& key )
         {
             return m_Buckets[ hash_value( key ) ];
         }
@@ -200,11 +212,11 @@ namespace cds { namespace container {
         //@cond
         const_iterator get_const_begin() const
         {
-            return const_iterator( const_cast<bucket_type const&>(m_Buckets[0]).begin(), m_Buckets, m_Buckets + bucket_count() );
+            return const_iterator( const_cast<internal_bucket_type const&>(m_Buckets[0]).begin(), m_Buckets, m_Buckets + bucket_count() );
         }
         const_iterator get_const_end() const
         {
-            return const_iterator( const_cast<bucket_type const&>(m_Buckets[bucket_count() - 1]).end(), m_Buckets + bucket_count() - 1, m_Buckets + bucket_count() );
+            return const_iterator( const_cast<internal_bucket_type const&>(m_Buckets[bucket_count() - 1]).end(), m_Buckets + bucket_count() - 1, m_Buckets + bucket_count() );
         }
         //@endcond
 
@@ -250,7 +262,7 @@ namespace cds { namespace container {
         template <typename Q>
         iterator insert( const Q& val )
         {
-            bucket_type& refBucket = bucket( val );
+            internal_bucket_type& refBucket = bucket( val );
             bucket_iterator it = refBucket.insert( val );
 
             if ( it != refBucket.end() ) {
@@ -268,9 +280,9 @@ namespace cds { namespace container {
         template <typename... Args>
         iterator emplace( Args&&... args )
         {
-            bucket_type& refBucket = bucket( value_type(std::forward<Args>(args)...));
-            bucket_iterator it = refBucket.emplace( std::forward<Args>(args)... );
-
+            typename internal_bucket_type::node_type * pNode = internal_bucket_type::alloc_node( std::forward<Args>( args )... );
+            internal_bucket_type& refBucket = bucket( internal_bucket_type::node_to_value( *pNode ));
+            bucket_iterator it = refBucket.insert_node( pNode );
             if ( it != refBucket.end() ) {
                 ++m_ItemCounter;
                 return iterator( it, &refBucket, m_Buckets + bucket_count() );
@@ -297,7 +309,7 @@ namespace cds { namespace container {
         template <typename Q>
         std::pair<iterator, bool> update( Q const& val, bool bAllowInsert = true )
         {
-            bucket_type& refBucket = bucket( val );
+            internal_bucket_type& refBucket = bucket( val );
             std::pair<bucket_iterator, bool> ret = refBucket.update( val, bAllowInsert );
 
             if ( ret.first != refBucket.end() ) {
@@ -328,7 +340,7 @@ namespace cds { namespace container {
         template <typename Q>
         iterator contains( Q const& key )
         {
-            bucket_type& refBucket = bucket( key );
+            internal_bucket_type& refBucket = bucket( key );
             bucket_iterator it = refBucket.contains( key );
             if ( it != refBucket.end() )
                 return iterator( it, &refBucket, m_Buckets + bucket_count() );
@@ -353,7 +365,7 @@ namespace cds { namespace container {
         template <typename Q, typename Less>
         iterator contains( Q const& key, Less pred )
         {
-            bucket_type& refBucket = bucket( key );
+            internal_bucket_type& refBucket = bucket( key );
             bucket_iterator it = refBucket.contains( key, pred );
             if ( it != refBucket.end() )
                 return iterator( it, &refBucket, m_Buckets + bucket_count() );

diff --git a/cds/container/michael_set_rcu.h b/cds/container/michael_set_rcu.h
index 0ea938b573a3208dfeb4bf36248a55f87b3aaf01..5afd9a5778a13de83cf83be20f7310cf015a6c6a 100644 (file)
--- a/cds/container/michael_set_rcu.h
+++ b/cds/container/michael_set_rcu.h
@@ -146,9 +146,6 @@ namespace cds { namespace container {
         typedef typename cds::opt::v::hash_selector< typename traits::hash >::type hash;
         typedef typename traits::item_counter item_counter;   ///< Item counter type
 
-        /// Bucket table allocator
-        typedef cds::details::Allocator< bucket_type, typename traits::allocator >  bucket_table_allocator;
-
         typedef typename bucket_type::rcu_lock   rcu_lock;   ///< RCU scoped lock
         typedef typename bucket_type::exempt_ptr exempt_ptr; ///< pointer to extracted node
         typedef typename bucket_type::raw_ptr    raw_ptr;    ///< Return type of \p get() member function and its derivatives
@@ -156,9 +153,26 @@ namespace cds { namespace container {
         static CDS_CONSTEXPR const bool c_bExtractLockExternal = bucket_type::c_bExtractLockExternal;
 
     protected:
-        item_counter    m_ItemCounter; ///< Item counter
-        hash            m_HashFunctor; ///< Hash functor
-        bucket_type *   m_Buckets;     ///< bucket table
+        //@cond
+        class internal_bucket_type: public bucket_type
+        {
+            typedef bucket_type base_class;
+        public:
+            using base_class::node_type;
+            using base_class::alloc_node;
+            using base_class::insert_node;
+            using base_class::node_to_value;
+        };
+
+        /// Bucket table allocator
+        typedef cds::details::Allocator< internal_bucket_type, typename traits::allocator >  bucket_table_allocator;
+
+        //@endcond
+
+    protected:
+        item_counter             m_ItemCounter; ///< Item counter
+        hash                     m_HashFunctor; ///< Hash functor
+        internal_bucket_type *   m_Buckets;     ///< bucket table
 
     private:
         //@cond
@@ -176,12 +190,12 @@ namespace cds { namespace container {
 
         /// Returns the bucket (ordered list) for \p key
         template <typename Q>
-        bucket_type&    bucket( Q const& key )
+        internal_bucket_type& bucket( Q const& key )
         {
             return m_Buckets[ hash_value( key ) ];
         }
         template <typename Q>
-        bucket_type const&    bucket( Q const& key ) const
+        internal_bucket_type const& bucket( Q const& key ) const
         {
             return m_Buckets[ hash_value( key ) ];
         }
@@ -280,11 +294,11 @@ namespace cds { namespace container {
         //@cond
         const_iterator get_const_begin() const
         {
-            return const_iterator( const_cast<bucket_type const&>(m_Buckets[0]).begin(), m_Buckets, m_Buckets + bucket_count() );
+            return const_iterator( const_cast<internal_bucket_type const&>(m_Buckets[0]).begin(), m_Buckets, m_Buckets + bucket_count() );
         }
         const_iterator get_const_end() const
         {
-            return const_iterator( const_cast<bucket_type const&>(m_Buckets[bucket_count() - 1]).end(), m_Buckets + bucket_count() - 1, m_Buckets + bucket_count() );
+            return const_iterator( const_cast<internal_bucket_type const&>(m_Buckets[bucket_count() - 1]).end(), m_Buckets + bucket_count() - 1, m_Buckets + bucket_count() );
         }
         //@endcond
 
@@ -306,7 +320,6 @@ namespace cds { namespace container {
             // GC and OrderedList::gc must be the same
             static_assert( std::is_same<gc, typename bucket_type::gc>::value, "GC and OrderedList::gc must be the same");
 
-            // atomicity::empty_item_counter is not allowed as a item counter
             static_assert( !std::is_same<item_counter, atomicity::empty_item_counter>::value,
                            "atomicity::empty_item_counter is not allowed as a item counter");
 
@@ -455,7 +468,8 @@ namespace cds { namespace container {
         template <typename... Args>
         bool emplace( Args&&... args )
         {
-            bool bRet = bucket( value_type(std::forward<Args>(args)...) ).emplace( std::forward<Args>(args)... );
+            typename internal_bucket_type::node_type * pNode = internal_bucket_type::alloc_node( std::forward<Args>( args )... );
+            bool bRet = bucket( internal_bucket_type::node_to_value( *pNode ) ).insert_node( pNode );
             if ( bRet )
                 ++m_ItemCounter;
             return bRet;