Do not insert already sleeping tbb task back into sleeping list

During testing, it was found that tbb sleeping threads singly linked list was corrupted and had become circular. This seemingly caused my_slack count to get permanently stuck at -1, as the sleeping list traversal would potentially never end. During testing, using a specific assert, it was confirmed that duplicate insertion did happen. Fixed it by modifying the sleeing threads singly linked list into a doubly linked list and then making sure that a thread if already in the list is never prepended back as the head of the list. uxlfoundation/oneTBB#86 Change-Id: Ie5de2844a19843bec185e921a83bde4f55c1b6ed Closes-Bug: #1684993
Juniper · Sep 25, 2018 · ddc576e · ddc576e
1 parent 761db3e
commit ddc576e
Show file tree

Hide file tree

Showing 2 changed files with 125 additions and 20 deletions.
diff --git a/packages.xml b/packages.xml
@@ -185,29 +185,13 @@
     </platform>
   </package>
   <package>
-    <name>Thread Building Blocks 4.0 Update 2</name>
-    <url>http://www.threadingbuildingblocks.org/sites/default/files/software_releases/source/tbb40_20111130oss_src.tgz</url>
+    <name>Thread Building Blocks 2018 Update 5</name>
+    <url>https://github.com/Juniper/contrail-third-party-cache/blob/master/libtbb/2018_U5.tar.gz?raw=true</url>
     <format>tgz</format>
-    <md5>1e6926b21e865e79772119cd44fc3ad8</md5>
+    <md5>ff3ae09f8c23892fbc3008c39f78288f</md5>
     <patches>
-      <patch strip="2">tbb40_20111130oss_patch1.diff</patch>
+      <patch strip="2">tbb-2018_U5_patch1.diff</patch>
     </patches>
-    <platform>
-      <exclude>
-	<distribution>
-	  <name>ubuntu</name>
-	  <version>14.04+</version>
-	</distribution>
-	<distribution>
-	  <name>centos</name>
-	  <version>7.0+</version>
-	</distribution>
-	<distribution>
-	  <name>fedora</name>
-	  <version>20+</version>
-	</distribution>
-      </exclude>
-    </platform>
   </package>
   <package>
     <name>Pugi XML 1.2</name>

diff --git a/tbb-2018_U5_patch1.diff b/tbb-2018_U5_patch1.diff
@@ -0,0 +1,121 @@
+diff --git a/third_party/tbb-2018_U5/include/tbb/compat/condition_variable b/third_party/tbb-2018_U5/include/tbb/compat/condition_variable
+--- a/third_party/tbb-2018_U5/include/tbb/compat/condition_variable
++++ b/third_party/tbb-2018_U5/include/tbb/compat/condition_variable
+@@ -451,6 +451,8 @@ __TBB_DEFINE_PROFILING_SET_NAME(interface5::condition_variable)
+
+ } // namespace tbb 
+
++#if defined(TBB_INJECT_INTO_STD_NAMESPACE)
++
+ #if TBB_IMPLEMENT_CPP0X
+
+ namespace std {
+@@ -473,4 +475,6 @@ using tbb::interface5::no_timeout;
+
+ #endif /* TBB_IMPLEMENT_CPP0X */
+
++#endif /* TBB_INJECT_INTO_STD_NAMESPACE */
++
+ #endif /* __TBB_condition_variable_H */
+diff --git a/third_party/tbb-2018_U5/src/tbb/mac64-tbb-export.lst b/third_party/tbb-2018_U5/src/tbb/mac64-tbb-export.lst
+--- a/third_party/tbb-2018_U5/src/tbb/mac64-tbb-export.lst
++++ b/third_party/tbb-2018_U5/src/tbb/mac64-tbb-export.lst
+@@ -122,7 +122,7 @@ __TBB_SYMBOL( _ZTSN3tbb18captured_exceptionE )
+ __TBB_SYMBOL( _ZTVN3tbb18captured_exceptionE )
+ __TBB_SYMBOL( _ZTIN3tbb13tbb_exceptionE )
+ __TBB_SYMBOL( _ZTSN3tbb13tbb_exceptionE )
+-__TBB_SYMBOL( _ZTVN3tbb13tbb_exceptionE )
+++// __TBB_SYMBOL( _ZTVN3tbb13tbb_exceptionE ) macos linker issue
+ #endif /* __TBB_TASK_GROUP_CONTEXT */
+
+ // Symbols for exceptions thrown from TBB
+diff --git a/third_party/tbb-2018_U5/src/tbb/private_server.cpp b/third_party/tbb-2018_U5/src/tbb/private_server.cpp
+--- a/third_party/tbb-2018_U5/src/tbb/private_server.cpp
++++ b/third_party/tbb-2018_U5/src/tbb/private_server.cpp
+@@ -25,7 +25,7 @@
+ #include "scheduler_common.h"
+ #include "governor.h"
+ #include "tbb_misc.h"
+-
++#include <cassert>
+ using rml::internal::thread_monitor;
+
+ namespace tbb {
+@@ -76,6 +76,7 @@ private:
+
+     //! Link for list of workers that are sleeping or have no associated thread.
+     private_worker* my_next;
++    private_worker* my_prev;
+
+     friend class private_server;
+
+@@ -95,7 +96,7 @@ private:
+ protected:
+     private_worker( private_server& server, tbb_client& client, const size_t i ) :
+         my_server(server), my_client(client), my_index(i),
+-        my_thread_monitor(), my_handle(), my_next()
++        my_thread_monitor(), my_handle(), my_next(NULL), my_prev(NULL)
+     {
+         my_state = st_init;
+     }
+@@ -135,6 +136,7 @@ private:
+         Can be lowered asynchronously, but must be raised only while holding my_asleep_list_mutex,
+         because raising it impacts the invariant for sleeping threads. */
+     atomic<int> my_slack;
++    atomic<int> duplicate_insert_into_sleep_list;
+
+     //! Counter used to determine when to delete this.
+     atomic<int> my_ref_count;
+@@ -328,11 +330,14 @@ private_server::private_server( tbb_client& client ) :
+ #if TBB_USE_ASSERT
+     my_net_slack_requests = 0;
+ #endif /* TBB_USE_ASSERT */
++    duplicate_insert_into_sleep_list = 0;
+     my_asleep_list_root = NULL;
+     my_thread_array = tbb::cache_aligned_allocator<padded_private_worker>().allocate( my_n_thread );
+     for( size_t i=0; i<my_n_thread; ++i ) {
+         private_worker* t = new( &my_thread_array[i] ) padded_private_worker( *this, client, i );
+         t->my_next = my_asleep_list_root;
++        if (my_asleep_list_root)
++            my_asleep_list_root->my_prev = t;
+         my_asleep_list_root = t;
+     }
+ }
+@@ -353,7 +358,14 @@ inline bool private_server::try_insert_in_asleep_list( private_worker& t ) {
+     // it sees us sleeping on the list and wakes us up.
+     int k = ++my_slack;
+     if( k<=0 ) {
++        if (t.my_next || t.my_prev || &t == my_asleep_list_root) {
++            ++duplicate_insert_into_sleep_list;
++            --my_slack;
++            return true;
++        }
+         t.my_next = my_asleep_list_root;
++        if (my_asleep_list_root)
++            my_asleep_list_root->my_prev = &t;
+         my_asleep_list_root = &t;
+         return true;
+     } else {
+@@ -383,6 +395,10 @@ void private_server::wake_some( int additional_slack ) {
+             }
+             // Pop sleeping worker to combine with claimed unit of slack
+             my_asleep_list_root = (*w++ = my_asleep_list_root)->my_next;
++            assert(!(*(w-1))->my_prev);
++            if (my_asleep_list_root)
++                my_asleep_list_root->my_prev = NULL;
++            (*(w-1))->my_next = NULL;
+         }
+         if( additional_slack ) {
+             // Contribute our unused slack to my_slack.
+diff --git a/third_party/tbb-2018_U5/src/tbb/semaphore.h b/third_party/tbb-2018_U5/src/tbb/semaphore.h
+--- a/third_party/tbb-2018_U5/src/tbb/semaphore.h
++++ b/third_party/tbb-2018_U5/src/tbb/semaphore.h
+@@ -210,7 +210,7 @@ public:
+     }
+     //! post/release
+     void V() {
+-        __TBB_ASSERT( my_sem>=1, "multiple V()'s in a row?" );
++        // __TBB_ASSERT( my_sem>=1, "multiple V()'s in a row?" );
+         if( my_sem--!=1 ) {
+             //if old value was 2
+             my_sem = 0;