From b2cbbebd9b6ff13e67e009f6c941d404c07cb6f9 Mon Sep 17 00:00:00 2001 From: khizmax Date: Sat, 8 Jul 2017 18:19:59 +0300 Subject: [PATCH] [TSan] Fixed data races --- cds/intrusive/cuckoo_set.h | 24 ++++++---- cds/intrusive/striped_set.h | 49 ++++++++++++--------- cds/intrusive/striped_set/striping_policy.h | 5 +++ 3 files changed, 47 insertions(+), 31 deletions(-) diff --git a/cds/intrusive/cuckoo_set.h b/cds/intrusive/cuckoo_set.h index 10a77026..951f5e7f 100644 --- a/cds/intrusive/cuckoo_set.h +++ b/cds/intrusive/cuckoo_set.h @@ -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 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 diff --git a/cds/intrusive/striped_set.h b/cds/intrusive/striped_set.h index 923fdfb3..00ab8674 100644 --- a/cds/intrusive/striped_set.h +++ b/cds/intrusive/striped_set.h @@ -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 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 @@ -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( 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 diff --git a/cds/intrusive/striped_set/striping_policy.h b/cds/intrusive/striped_set/striping_policy.h index b841a17c..40479632 100644 --- a/cds/intrusive/striped_set/striping_policy.h +++ b/cds/intrusive/striped_set/striping_policy.h @@ -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; } }; -- 2.34.1