[TSan] Fixed data races
authorkhizmax <libcds.dev@gmail.com>
Sat, 8 Jul 2017 15:19:59 +0000 (18:19 +0300)
committerkhizmax <libcds.dev@gmail.com>
Sat, 8 Jul 2017 15:19:59 +0000 (18:19 +0300)
cds/intrusive/cuckoo_set.h
cds/intrusive/striped_set.h
cds/intrusive/striped_set/striping_policy.h

index 10a77026d1afa6b876a8d8362782c8cc1ed81b9c..951f5e7fd652f5d4e5f00d0be28b0986b6815469 100644 (file)
@@ -1943,9 +1943,9 @@ namespace cds { namespace intrusive {
     protected:
         bucket_entry *      m_BucketTable[ c_nArity ] ; ///< Bucket tables
 
-        size_t              m_nBucketMask           ;   ///< Hash bitmask; bucket table size minus 1.
-        unsigned int const  m_nProbesetSize         ;   ///< Probe set size
-        unsigned int const  m_nProbesetThreshold    ;   ///< Probe set threshold
+        atomics::atomic<size_t> m_nBucketMask           ;   ///< Hash bitmask; bucket table size minus 1.
+        unsigned int const      m_nProbesetSize         ;   ///< Probe set size
+        unsigned int const      m_nProbesetThreshold    ;   ///< Probe set threshold
 
         hash            m_Hash              ;   ///< Hash functor tuple
         mutex_policy    m_MutexPolicy       ;   ///< concurrent access policy
@@ -1984,7 +1984,7 @@ namespace cds { namespace intrusive {
         bucket_entry& bucket( unsigned int nTable, size_t nHash )
         {
             assert( nTable < c_nArity );
-            return m_BucketTable[nTable][nHash & m_nBucketMask];
+            return m_BucketTable[nTable][nHash & m_nBucketMask.load( atomics::memory_order_relaxed ) ];
         }
 
         static void store_hash( node_type * pNode, size_t * pHashes )
@@ -2001,7 +2001,7 @@ namespace cds { namespace intrusive {
         {
             assert( cds::beans::is_power2( nSize ));
 
-            m_nBucketMask = nSize - 1;
+            m_nBucketMask.store( nSize - 1, atomics::memory_order_release );
             bucket_table_allocator alloc;
             for ( unsigned int i = 0; i < c_nArity; ++i )
                 m_BucketTable[i] = alloc.NewArray( nSize );
@@ -2017,7 +2017,7 @@ namespace cds { namespace intrusive {
         }
         void free_bucket_tables()
         {
-            free_bucket_tables( m_BucketTable, m_nBucketMask + 1 );
+            free_bucket_tables( m_BucketTable, m_nBucketMask.load( atomics::memory_order_relaxed ) + 1 );
         }
 
         static CDS_CONSTEXPR unsigned int const c_nUndefTable = (unsigned int) -1;
@@ -2155,7 +2155,7 @@ namespace cds { namespace intrusive {
         {
             m_Stat.onResizeCall();
 
-            size_t nOldCapacity = bucket_count();
+            size_t nOldCapacity = bucket_count( atomics::memory_order_acquire );
             bucket_entry *      pOldTable[ c_nArity ];
             {
                 scoped_resize_lock guard( m_MutexPolicy );
@@ -2769,7 +2769,7 @@ namespace cds { namespace intrusive {
 
             for ( unsigned int i = 0; i < c_nArity; ++i ) {
                 bucket_entry * pEntry = m_BucketTable[i];
-                bucket_entry * pEnd = pEntry + m_nBucketMask + 1;
+                bucket_entry * pEnd = pEntry + m_nBucketMask.load( atomics::memory_order_relaxed ) + 1;
                 for ( ; pEntry != pEnd ; ++pEntry ) {
                     pEntry->clear( [&oDisposer]( node_type * pNode ){ oDisposer( node_traits::to_value_ptr( pNode )) ; } );
                 }
@@ -2798,8 +2798,14 @@ namespace cds { namespace intrusive {
         */
         size_t bucket_count() const
         {
-            return m_nBucketMask + 1;
+            return m_nBucketMask.load( atomics::memory_order_relaxed ) + 1;
         }
+        //@cond
+        size_t bucket_count( atomics::memory_order load_mo ) const
+        {
+            return m_nBucketMask.load( load_mo ) + 1;
+        }
+        //@endcond
 
         /// Returns lock array size
         size_t lock_count() const
index 923fdfb358dd1fd35836368920f65fca2c9c19c5..00ab867438ba46beeb16ae9f63b116048243508f 100644 (file)
@@ -326,10 +326,10 @@ namespace cds { namespace intrusive {
         typedef cds::details::Allocator< bucket_type, allocator_type > bucket_allocator;  ///< bucket allocator type based on allocator_type
 
     protected:
-        bucket_type *   m_Buckets       ;   ///< Bucket table
-        size_t          m_nBucketMask   ;   ///< Bucket table size - 1. m_nBucketMask + 1 should be power of two.
-        item_counter    m_ItemCounter   ;   ///< Item counter
-        hash            m_Hash          ;   ///< Hash functor
+        bucket_type *           m_Buckets;      ///< Bucket table
+        atomics::atomic<size_t> m_nBucketMask;  ///< Bucket table size - 1. m_nBucketMask + 1 should be power of two.
+        item_counter            m_ItemCounter;  ///< Item counter
+        hash                    m_Hash;         ///< Hash functor
 
         mutex_policy    m_MutexPolicy   ;   ///< Mutex policy
         resizing_policy m_ResizingPolicy;   ///< Resizing policy
@@ -354,7 +354,7 @@ namespace cds { namespace intrusive {
         void alloc_bucket_table( size_t nSize )
         {
             assert( cds::beans::is_power2( nSize ));
-            m_nBucketMask = nSize - 1;
+            m_nBucketMask.store( nSize - 1, atomics::memory_order_release );
             m_Buckets = bucket_allocator().NewArray( nSize );
         }
 
@@ -371,7 +371,7 @@ namespace cds { namespace intrusive {
 
         bucket_type * bucket( size_t nHash ) const CDS_NOEXCEPT
         {
-            return m_Buckets + (nHash & m_nBucketMask);
+            return m_Buckets + (nHash & m_nBucketMask.load( atomics::memory_order_relaxed ));
         }
 
         template <typename Q, typename Func>
@@ -421,12 +421,11 @@ namespace cds { namespace intrusive {
 
         void resize()
         {
-            size_t nOldCapacity = bucket_count();
-            size_t volatile& refBucketMask = m_nBucketMask;
+            size_t nOldCapacity = bucket_count( atomics::memory_order_acquire );
 
             scoped_resize_lock al( m_MutexPolicy );
             if ( al.success()) {
-                if ( nOldCapacity != refBucketMask + 1 ) {
+                if ( nOldCapacity != bucket_count( atomics::memory_order_acquire ) ) {
                     // someone resized already
                     return;
                 }
@@ -441,21 +440,21 @@ namespace cds { namespace intrusive {
         /// Default ctor. The initial capacity is 16.
         StripedSet()
             : m_Buckets( nullptr )
-        , m_nBucketMask( c_nMinimalCapacity - 1 )
-        , m_MutexPolicy( c_nMinimalCapacity )
+            , m_nBucketMask( c_nMinimalCapacity - 1 )
+            , m_MutexPolicy( c_nMinimalCapacity )
         {
-            alloc_bucket_table( m_nBucketMask + 1 );
+            alloc_bucket_table( bucket_count() );
         }
 
         /// Ctor with initial capacity specified
         StripedSet(
             size_t nCapacity    ///< Initial size of bucket table and lock array. Must be power of two, the minimum is 16.
         )
-        : m_Buckets( nullptr )
-        , m_nBucketMask( calc_init_capacity(nCapacity) - 1 )
-        , m_MutexPolicy( m_nBucketMask + 1 )
+            : m_Buckets( nullptr )
+            , m_nBucketMask( calc_init_capacity(nCapacity) - 1 )
+            , m_MutexPolicy( bucket_count() )
         {
-            alloc_bucket_table( m_nBucketMask + 1 );
+            alloc_bucket_table( bucket_count() );
         }
 
         /// Ctor with resizing policy (copy semantics)
@@ -468,10 +467,10 @@ namespace cds { namespace intrusive {
         )
         : m_Buckets( nullptr )
         , m_nBucketMask( ( nCapacity ? calc_init_capacity(nCapacity) : c_nMinimalCapacity ) - 1 )
-        , m_MutexPolicy( m_nBucketMask + 1 )
+        , m_MutexPolicy( bucket_count() )
         , m_ResizingPolicy( resizingPolicy )
         {
-            alloc_bucket_table( m_nBucketMask + 1 );
+            alloc_bucket_table( bucket_count() );
         }
 
         /// Ctor with resizing policy (move semantics)
@@ -485,16 +484,16 @@ namespace cds { namespace intrusive {
         )
         : m_Buckets( nullptr )
         , m_nBucketMask( ( nCapacity ? calc_init_capacity(nCapacity) : c_nMinimalCapacity ) - 1 )
-        , m_MutexPolicy( m_nBucketMask + 1 )
+        , m_MutexPolicy( bucket_count() )
         , m_ResizingPolicy( std::forward<resizing_policy>( resizingPolicy ))
         {
-            alloc_bucket_table( m_nBucketMask + 1 );
+            alloc_bucket_table( bucket_count() );
         }
 
         /// Destructor destroys internal data
         ~StripedSet()
         {
-            free_bucket_table( m_Buckets, m_nBucketMask + 1 );
+            free_bucket_table( m_Buckets, bucket_count() );
         }
 
     public:
@@ -878,8 +877,14 @@ namespace cds { namespace intrusive {
         */
         size_t bucket_count() const
         {
-            return m_nBucketMask + 1;
+            return m_nBucketMask.load( atomics::memory_order_relaxed ) + 1;
         }
+        //@cond
+        size_t bucket_count( atomics::memory_order load_mo ) const
+        {
+            return m_nBucketMask.load( load_mo ) + 1;
+        }
+        //@endcond
 
         /// Returns lock array size
         size_t lock_count() const
index b841a17c9a404a7c2d950c4b102e2b9a026b1e9b..4047963254ccce5e1aa578dd76e81e4b5d1bd257 100644 (file)
@@ -191,7 +191,12 @@ namespace cds { namespace intrusive { namespace striped_set {
         struct lock_array_disposer {
             void operator()( lock_array_type * pArr )
             {
+                // Seems, there is a false positive in std::shared_ptr deallocation in uninstrumented libc++
+                // see, for example, https://groups.google.com/forum/#!topic/thread-sanitizer/eHu4dE_z7Cc
+                // https://reviews.llvm.org/D21609
+                CDS_TSAN_ANNOTATE_IGNORE_WRITES_BEGIN;
                 lock_array_allocator().Delete( pArr );
+                CDS_TSAN_ANNOTATE_IGNORE_WRITES_END;
             }
         };