From cae6c4f7622724d3505f303e5e8d4d6a5a840bd5 Mon Sep 17 00:00:00 2001
From: khizmax <libcds.dev@gmail.com>
Date: Thu, 30 Apr 2015 23:08:09 +0300
Subject: [PATCH] TSan exam: fixed data races and false positives in queues

---
 cds/container/msqueue.h                            |  3 +++
 cds/intrusive/fcqueue.h                            |  9 +++++++++
 cds/intrusive/segmented_queue.h                    | 12 ++++++++++--
 cds/memory/michael/allocator.h                     |  4 ++++
 tests/unit/queue/intrusive_queue_reader_writer.cpp |  2 ++
 5 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/cds/container/msqueue.h b/cds/container/msqueue.h
index 1d355e97..3292ff91 100644
--- a/cds/container/msqueue.h
+++ b/cds/container/msqueue.h
@@ -117,7 +117,10 @@ namespace cds { namespace container {
             {
                 void operator ()( node_type * pNode )
                 {
+                    // TSan false positive possible
+                    CDS_TSAN_ANNOTATE_IGNORE_WRITES_BEGIN;
                     cxx_allocator().Delete( pNode );
+                    CDS_TSAN_ANNOTATE_IGNORE_WRITES_END;
                 }
             };
 
diff --git a/cds/intrusive/fcqueue.h b/cds/intrusive/fcqueue.h
index 642e4fc0..86287365 100644
--- a/cds/intrusive/fcqueue.h
+++ b/cds/intrusive/fcqueue.h
@@ -260,6 +260,9 @@ 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_enq:
                 assert( pRec->pVal );
@@ -282,11 +285,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() ) {
@@ -301,6 +308,8 @@ namespace cds { namespace intrusive {
                     break;
                 }
             }
+
+            CDS_TSAN_ANNOTATE_IGNORE_RW_END;
         }
         //@endcond
 
diff --git a/cds/intrusive/segmented_queue.h b/cds/intrusive/segmented_queue.h
index c8e1430d..b3f244c4 100644
--- a/cds/intrusive/segmented_queue.h
+++ b/cds/intrusive/segmented_queue.h
@@ -352,7 +352,7 @@ namespace cds { namespace intrusive {
                 m_Stat.onSegmentCreated();
 
                 if ( m_List.empty() )
-                    m_pHead.store( pNew, memory_model::memory_order_relaxed );
+                    m_pHead.store( pNew, memory_model::memory_order_release );
                 m_List.push_back( *pNew );
                 m_pTail.store( pNew, memory_model::memory_order_release );
                 return guard.assign( pNew );
@@ -411,13 +411,21 @@ namespace cds { namespace intrusive {
 
             segment * allocate_segment()
             {
-                return segment_allocator().NewBlock( sizeof(segment) + sizeof(cell) * m_nQuasiFactor,
+                // TSan: release barrier will be issued when the segment will link to the list of segments
+                CDS_TSAN_ANNOTATE_IGNORE_WRITES_BEGIN;
+                segment * p = segment_allocator().NewBlock( sizeof(segment) + sizeof(cell) * m_nQuasiFactor,
                     quasi_factor() );
+                CDS_TSAN_ANNOTATE_IGNORE_WRITES_END;
+                return p;
             }
 
             static void free_segment( segment * pSegment )
             {
+                // TSan: deallocating is called inside SMR reclamation cycle
+                // so necessary barriers have been already issued
+                CDS_TSAN_ANNOTATE_IGNORE_WRITES_BEGIN;
                 segment_allocator().Delete( pSegment );
+                CDS_TSAN_ANNOTATE_IGNORE_WRITES_END;
             }
 
             static void retire_segment( segment * pSegment )
diff --git a/cds/memory/michael/allocator.h b/cds/memory/michael/allocator.h
index 5b14d48c..d82d10cd 100644
--- a/cds/memory/michael/allocator.h
+++ b/cds/memory/michael/allocator.h
@@ -1450,6 +1450,9 @@ namespace michael {
 
             static_assert( (sizeof(processor_heap) % c_nAlignment) == 0, "sizeof(processor_heap) error" );
 
+            // TSan false positive: a new descriptor will be linked further with release fence
+            CDS_TSAN_ANNOTATE_IGNORE_WRITES_BEGIN;
+
             pDesc = new( m_AlignedHeap.alloc( szTotal, c_nAlignment ) ) processor_desc;
 
             pDesc->pageHeaps = reinterpret_cast<page_heap *>( pDesc + 1 );
@@ -1474,6 +1477,7 @@ namespace michael {
                 else
                     pProcHeap->nPageIdx = pProcHeap->pSizeClass->nSBSizeIdx;
             }
+            CDS_TSAN_ANNOTATE_IGNORE_WRITES_END;
             return pDesc;
         }
 
diff --git a/tests/unit/queue/intrusive_queue_reader_writer.cpp b/tests/unit/queue/intrusive_queue_reader_writer.cpp
index b5ad38c4..a4f0e734 100644
--- a/tests/unit/queue/intrusive_queue_reader_writer.cpp
+++ b/tests/unit/queue/intrusive_queue_reader_writer.cpp
@@ -168,12 +168,14 @@ namespace queue {
                 while ( true ) {
                     typename Queue::value_type * p = m_Queue.pop();
                     if ( p ) {
+                        CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;
                         p->nConsumer = m_nThreadNo;
                         ++m_nPopped;
                         if ( p->nWriterNo < nTotalWriters )
                             m_WriterData[ p->nWriterNo ].push_back( p->nNo );
                         else
                             ++m_nBadWriter;
+                        CDS_TSAN_ANNOTATE_IGNORE_RW_END;
                     }
                     else {
                         ++m_nPopEmpty;
-- 
2.34.1