TSan exam: fixed flat combining data race; eliminated false positive warnings

diff --git a/cds/algo/flat_combining.h b/cds/algo/flat_combining.h
index 1a3d02d0b0f4bcb631aa608d696ed04992f36438..b239ef4218c61d2d48cab914c968a2cbc3094c4f 100644 (file)
--- a/cds/algo/flat_combining.h
+++ b/cds/algo/flat_combining.h
@@ -95,7 +95,7 @@ namespace cds { namespace algo {
         struct publication_record {
             atomics::atomic<unsigned int>    nRequest;   ///< Request field (depends on data structure)
             atomics::atomic<unsigned int>    nState;     ///< Record state: inactive, active, removed
-            unsigned int                        nAge;       ///< Age of the record
+            atomics::atomic<unsigned int>    nAge;       ///< Age of the record
             atomics::atomic<publication_record *> pNext; ///< Next record in publication list
             void *                              pOwner;    ///< [internal data] Pointer to \ref kernel object that manages the publication list
 
@@ -338,7 +338,7 @@ namespace cds { namespace algo {
             void release_record( publication_record_type * pRec )
             {
                 assert( pRec->is_done() );
-                pRec->nRequest.store( req_EmptyRecord, memory_model::memory_order_relaxed );
+                pRec->nRequest.store( req_EmptyRecord, memory_model::memory_order_release );
                 m_Stat.onReleasePubRecord();
             }
 
@@ -565,7 +565,7 @@ namespace cds { namespace algo {
             {
                 assert( pRec->nState.load( memory_model::memory_order_relaxed ) == inactive );
 
-                pRec->nAge = m_nCount;
+                pRec->nAge.store( m_nCount, memory_model::memory_order_release );
                 pRec->nState.store( active, memory_model::memory_order_release );
 
                 // Insert record to publication list
@@ -673,7 +673,7 @@ namespace cds { namespace algo {
                     switch ( p->nState.load( memory_model::memory_order_acquire )) {
                         case active:
                             if ( p->op() >= req_Operation ) {
-                                p->nAge = nCurAge;
+                                p->nAge.store( nCurAge, memory_model::memory_order_release );
                                 owner.fc_apply( static_cast<publication_record_type *>(p) );
                                 operation_done( *p );
                                 bOpDone = true;
@@ -741,7 +741,9 @@ namespace cds { namespace algo {
                 // Thinning publication list
                 publication_record * pPrev = nullptr;
                 for ( publication_record * p = m_pHead; p; ) {
-                    if ( p->nState.load( memory_model::memory_order_acquire ) == active && p->nAge + m_nCompactFactor < nCurAge ) {
+                    if ( p->nState.load( memory_model::memory_order_acquire ) == active 
+                      && p->nAge.load( memory_model::memory_order_acquire ) + m_nCompactFactor < nCurAge ) 
+                    {
                         if ( pPrev ) {
                             publication_record * pNext = p->pNext.load( memory_model::memory_order_acquire );
                             if ( pPrev->pNext.compare_exchange_strong( p, pNext,

diff --git a/cds/container/fcdeque.h b/cds/container/fcdeque.h
index 55dbb33b8718bc8fd9ebd58cf1cc1b0ad61efc15..9d75864a992b69e54416acad4ae5fbb10db072bb 100644 (file)
--- a/cds/container/fcdeque.h
+++ b/cds/container/fcdeque.h
@@ -356,6 +356,9 @@ namespace cds { namespace container {
         {
             assert( pRec );
 
+            // this function is called under FC mutex, so switch TSan off
+            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
+
             switch ( pRec->op() ) {
             case op_push_front:
                 assert( pRec->pValPush );
@@ -397,12 +400,17 @@ namespace cds { namespace container {
                 assert(false);
                 break;
             }
+            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
 
         /// Batch-processing flat combining
         void fc_process( typename fc_kernel::iterator itBegin, typename fc_kernel::iterator itEnd )
         {
             typedef typename fc_kernel::iterator fc_iterator;
+
+            // this function is called under FC mutex, so switch TSan off
+            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
+
             for ( fc_iterator it = itBegin, itPrev = itEnd; it != itEnd; ++it ) {
                 switch ( it->op() ) {
                 case op_push_front:
@@ -451,6 +459,7 @@ namespace cds { namespace container {
                     break;
                 }
             }
+            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
         //@endcond
 

diff --git a/cds/container/fcstack.h b/cds/container/fcstack.h
index 3d3d5e4df05620ac1c9134e84a93a26ced499fc4..0f6aeb4d7c62b2a3f8d3b371e66513b443c67049 100644 (file)
--- a/cds/container/fcstack.h
+++ b/cds/container/fcstack.h
@@ -269,6 +269,8 @@ namespace cds { namespace container {
         {
             assert( pRec );
 
+            // this function is called under FC mutex, so switch TSan off
+            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
             switch ( pRec->op() ) {
             case op_push:
                 assert( pRec->pValPush );
@@ -294,11 +296,14 @@ namespace cds { namespace container {
                 assert(false);
                 break;
             }
+            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
 
         /// Batch-processing flat combining
         void fc_process( typename fc_kernel::iterator itBegin, typename fc_kernel::iterator itEnd )
         {
+            // this function is called under FC mutex, so switch TSan off
+            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
             typedef typename fc_kernel::iterator fc_iterator;
             for ( fc_iterator it = itBegin, itPrev = itEnd; it != itEnd; ++it ) {
                 switch ( it->op() ) {
@@ -312,6 +317,7 @@ namespace cds { namespace container {
                     break;
                 }
             }
+            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
         //@endcond
 

diff --git a/cds/intrusive/fcstack.h b/cds/intrusive/fcstack.h
index f03dd3fee0fa33e58c8d2133211d3971497a2b54..3bdf12423adcbdc702597ed01b8dfbcfd07526a5 100644 (file)
--- a/cds/intrusive/fcstack.h
+++ b/cds/intrusive/fcstack.h
@@ -247,6 +247,8 @@ namespace cds { namespace intrusive {
         {
             assert( pRec );
 
+            // this function is called under FC mutex, so switch TSan off
+            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
             switch ( pRec->op() ) {
             case op_push:
                 assert( pRec->pVal );
@@ -269,11 +271,15 @@ namespace cds { namespace intrusive {
                 assert(false);
                 break;
             }
+            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
 
         /// Batch-processing flat combining
         void fc_process( typename fc_kernel::iterator itBegin, typename fc_kernel::iterator itEnd )
         {
+            // this function is called under FC mutex, so switch TSan off
+            CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
+
             typedef typename fc_kernel::iterator fc_iterator;
             for ( fc_iterator it = itBegin, itPrev = itEnd; it != itEnd; ++it ) {
                 switch ( it->op() ) {
@@ -286,6 +292,8 @@ namespace cds { namespace intrusive {
                     break;
                 }
             }
+
+            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
         //@endcond
 

diff --git a/src/hp_gc.cpp b/src/hp_gc.cpp
index 1516679059732e97a17850696434368dee78c63c..ae97cb3ea31ab5ac414cc4bda499cc5e5c04f1c4 100644 (file)
--- a/src/hp_gc.cpp
+++ b/src/hp_gc.cpp
@@ -321,7 +321,15 @@ namespace cds { namespace gc {
                 details::retired_vector& dest = pThis->m_arrRetired;
                 assert( !dest.isFull());
                 details::retired_vector::iterator itRetired = src.begin();
+
+                // TSan can issue a warning here:
+                //  read src.m_nSize in src.end()
+                //  write src.m_nSize in src.clear() 
+                // This is false positive since we own hprec
+                CDS_TSAN_ANNOTATE_IGNORE_READS_BEGIN;
                 details::retired_vector::iterator itRetiredEnd = src.end();
+                CDS_TSAN_ANNOTATE_IGNORE_READS_END;
+
                 while ( itRetired != itRetiredEnd ) {
                     dest.push( *itRetired );
                     if ( dest.isFull()) {
@@ -330,7 +338,11 @@ namespace cds { namespace gc {
                     }
                     ++itRetired;
                 }
+
+                // TSan: write src.m_nSize, see a comment above
+                CDS_TSAN_ANNOTATE_IGNORE_WRITES_BEGIN;
                 src.clear();
+                CDS_TSAN_ANNOTATE_IGNORE_WRITES_END;
 
                 hprec->m_bFree.store(true, atomics::memory_order_release);
                 hprec->m_idOwner.store( nullThreadId, atomics::memory_order_release );