Merge pull request #304 from insertinterestingnamehere/tsan

Thread Sanitizer Fixes
sandialabs · Oct 21, 2024 · 3ec21c0 · 3ec21c0
2 parents 2345ecf + e3e1f8f
commit 3ec21c0
Show file tree

Hide file tree

Showing 9 changed files with 132 additions and 74 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -27,7 +27,7 @@ jobs:
           make -j2
           make tests -j2
       - run:
-          command: timeout --foreground -k 10s 2m make check
+          command: timeout --foreground -k 10s 2m make check || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
           no_output_timeout: 180s
 
   arm_clang:
@@ -58,7 +58,7 @@ jobs:
           make -j2
           make tests -j2
       - run:
-          command: timeout --foreground -k 10s 2m make check
+          command: timeout --foreground -k 10s 2m make check || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
           no_output_timeout: 180s
 
   arm_sanitizers:
@@ -96,7 +96,10 @@ jobs:
           make -j2
           make tests -j2
       - run:
-          command: timeout --foreground -k 10s 4m make check
+          command: |
+              export QTHREADS_DIR="$(pwd)"
+              if [[ "<< parameters.sanitizer >>" == "thread" ]]; then cd test/basics; fi
+              timeout --foreground -k 10s 4m make check || ( cd $QTHREADS_DIR && cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
           no_output_timeout: 120s
 
   arm_acfl:
@@ -134,7 +137,7 @@ jobs:
       - run:
           command: |
               export PATH=$PATH:/opt/arm/arm-linux-compiler-24.04_Ubuntu-22.04/bin
-              timeout --foreground -k 10s 4m make check
+              timeout --foreground -k 10s 4m make check || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
           no_output_timeout: 180s
 
   nvc:
@@ -173,7 +176,7 @@ jobs:
           make tests -j2
       - run:
           command: |
-              timeout --foreground -k 10s 4m make check
+              timeout --foreground -k 10s 4m make check || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
           no_output_timeout: 180s
 
   musl:
@@ -197,7 +200,7 @@ jobs:
           make -j2
           make tests -j2
       - run:
-          command: make check
+          command: make check  || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
           no_output_timeout: 180s
 
 workflows:
@@ -238,6 +241,24 @@ workflows:
               - scheduler: distrib
                 topology: hwloc
                 sanitizer: memory
+              - scheduler: sherwood
+                topology: 'no'
+                sanitizer: thread
+              - scheduler: sherwood
+                topology: hwloc
+                sanitizer: thread
+              - scheduler: sherwood
+                topology: binders
+                sanitizer: thread
+              - scheduler: distrib
+                topology: 'no'
+                sanitizer: thread
+              - scheduler: distrib
+                topology: hwloc
+                sanitizer: thread
+              - scheduler: distrib
+                topology: binders
+                sanitizer: thread
       - arm_acfl:
           matrix:
             parameters:

diff --git a/.cirrus.yml b/.cirrus.yml
@@ -45,7 +45,7 @@ osx_m1_task:
     # commented example for how to get a backtrace from CI usign lldb on OSX:
     #echo "settings set target.process.stop-on-exec false" > ~/.lldbinit
     #QT_NUM_SHEPHERDS=2 QT_NUM_WORKERS_PER_SHEPHERD=1 lldb bash --batch --one-line 'process launch' --one-line-on-crash 'bt' --one-line-on-crash 'quit' -- test/basics/hello_world
-    gtimeout --foreground 3m make check
+    gtimeout --foreground 3m make check || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
 
 freebsd_task:
   freebsd_instance:
@@ -68,7 +68,7 @@ freebsd_task:
     make -j$CIRRUS_CPU
     make tests -j$CIRRUS_CPU
   test_script: |
-    gtimeout --foreground -k 10s 2m make check
+    gtimeout --foreground -k 10s 2m make check || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
 
 arm_linux_task:
   arm_container:
@@ -112,7 +112,7 @@ arm_linux_task:
     make -j$CIRRUS_CPU
     make tests -j$CIRRUS_CPU
   test_script: |
-    timeout --foreground -k 10s 2m make check
+    timeout --foreground -k 10s 5m make check || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
 
 arm_linux_clang_task:
   arm_container:
@@ -168,5 +168,5 @@ arm_linux_clang_task:
     make -j$CIRRUS_CPU
     make tests -j$CIRRUS_CPU
   test_script: |
-    timeout --foreground -k 10s 2m make check
+    timeout --foreground -k 10s 5m make check  || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
 
diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml
@@ -35,7 +35,7 @@ jobs:
         make -j2
         make tests -j2
     - name: make check
-      run: timeout -k 10s --foreground 3m make check
+      run: timeout -k 10s --foreground 3m make check || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
       timeout-minutes: 4
 
   linux-clang:
@@ -77,7 +77,7 @@ jobs:
         make -j2
         make tests -j2
     - name: make check
-      run: timeout -k 10s --foreground 6m make check
+      run: timeout -k 10s --foreground 6m make check || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
       timeout-minutes: 7
 
   linux-icx:
@@ -116,7 +116,7 @@ jobs:
     - name: make check
       run: |
         source /opt/intel/oneapi/setvars.sh
-        timeout -k 10s --foreground 6m make check
+        timeout -k 10s --foreground 6m make check || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
       timeout-minutes: 7
 
   linux-icc:
@@ -157,7 +157,7 @@ jobs:
     - name: make check
       run: |
         source /opt/intel/oneapi/setvars.sh
-        timeout -k 10s --foreground 6m make check
+        timeout -k 10s --foreground 6m make check || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
       timeout-minutes: 7
 
   linux-aocc:
@@ -192,7 +192,7 @@ jobs:
         make tests -j2
     - name: make check
       run: |
-        timeout -k 10s --foreground 6m make check
+        timeout -k 10s --foreground 6m make check || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
       timeout-minutes: 7
 
   mac:
@@ -228,7 +228,7 @@ jobs:
         # commented example for how to get a backtrace from CI usign lldb on OSX:
         #echo "settings set target.process.stop-on-exec false" > ~/.lldbinit
         #QT_NUM_SHEPHERDS=2 QT_NUM_WORKERS_PER_SHEPHERD=1 lldb bash --batch --one-line 'process launch' --one-line-on-crash 'bt' --one-line-on-crash 'quit' -- test/basics/hello_world
-        gtimeout -k 10s --foreground 8m make check
+        gtimeout -k 10s --foreground 8m make check || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
       timeout-minutes: 9
 
   sanitizers:
@@ -245,6 +245,14 @@ jobs:
             topology: hwloc
           - sanitizer: memory
             topology: binders
+          - sanitizer: thread
+            scheduler: sherwood
+          - sanitizer: thread
+            scheduler: distrib
+          - sanitizer: thread
+            topology: hwloc
+          - sanitizer: thread
+            topology: binders
     env:
       CC: clang-19
       CXX: clang++-19
@@ -276,7 +284,10 @@ jobs:
         make -j2
         make tests -j2
     - name: make check
-      run: timeout -k 10s --foreground 8m make check
+      run: |
+        export QTHREADS_DIR="$(pwd)"
+        if [[ "${{ matrix.sanitizer }}" == "thread" ]]; then cd test/basics; fi
+        timeout -k 10s --foreground 8m make check || ( cd $QTHREADS_DIR && cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
       timeout-minutes: 9
 
   linux-thorough:
@@ -319,7 +330,7 @@ jobs:
         make -j2
         make tests -j2
     - name: make check
-      run: timeout -k 10s --foreground 6m make check
+      run: timeout -k 10s --foreground 6m make check || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
       timeout-minutes: 7
 
   clang-format:

diff --git a/include/qt_queue.h b/include/qt_queue.h
@@ -9,24 +9,25 @@
  */
 
 typedef struct qthread_queue_node_s {
-  struct qthread_queue_node_s *next;
-  qthread_t *thread;
+  struct qthread_queue_node_s *_Atomic next;
+  qthread_t *_Atomic thread;
 } qthread_queue_node_t;
 
 typedef struct qthread_queue_NEMESIS_s {
   /* The First Cacheline */
-  void *head;
-  void *tail;
-  uint8_t pad1[CACHELINE_WIDTH - (2 * sizeof(void *))];
+  void *_Atomic head;
+  uint8_t pad1[CACHELINE_WIDTH - sizeof(void *)];
+  void *_Atomic tail;
+  uint8_t pad2[CACHELINE_WIDTH - sizeof(void *)];
   /* The Second Cacheline */
-  aligned_t length;
+  _Atomic aligned_t length;
   void *shadow_head;
-  uint8_t pad2[CACHELINE_WIDTH - sizeof(void *) - sizeof(aligned_t)];
+  uint8_t pad3[CACHELINE_WIDTH - sizeof(void *) - sizeof(aligned_t)];
 } qthread_queue_NEMESIS_t;
 
 typedef struct qthread_queue_nosync_s {
-  qthread_queue_node_t *head;
-  qthread_queue_node_t *tail;
+  qthread_queue_node_t *_Atomic head;
+  qthread_queue_node_t *_Atomic tail;
 } qthread_queue_nosync_t;
 
 typedef struct qthread_queue_capped_s {

diff --git a/include/qt_shepherd_innards.h b/include/qt_shepherd_innards.h
@@ -36,7 +36,7 @@ struct qthread_worker_s {
   qthread_shepherd_t *shepherd;
   struct qthread_s **nostealbuffer;
   struct qthread_s **stealbuffer;
-  qthread_t *current;
+  qthread_t *_Atomic current;
   qthread_worker_id_t unique_id;
   qthread_worker_id_t worker_id;
   qthread_worker_id_t packed_worker_id;

diff --git a/src/qthread.c b/src/qthread.c
@@ -334,7 +334,7 @@ static void *qthread_master(void *arg) {
   qt_threadqueue_t *threadqueue;
   qt_threadqueue_private_t *localqueue = NULL;
   qthread_t *t;
-  qthread_t **current;
+  qthread_t *_Atomic *current;
   int done = 0;
 
   assert(me != NULL);
@@ -444,8 +444,9 @@ static void *qthread_master(void *arg) {
 #endif
         qthread_exec(t, &my_context);
 
-        t = *current;    // necessary for direct-swap sanity
-        *current = NULL; // neessary for "queue sanity"
+        t = *current; // necessary for direct-swap sanity
+        atomic_store_explicit(
+          current, NULL, memory_order_relaxed); // neessary for "queue sanity"
 
         /* now clean up, based on the thread's state */
         switch (atomic_load_explicit(&t->thread_state, memory_order_relaxed)) {
@@ -1340,7 +1341,8 @@ qthread_readstate(const enum introspective_state type) { /*{{{ */
         sum += qt_threadqueue_advisory_queuelen(sheps[s].ready);
         qthread_worker_t const *wkrs = sheps[s].workers;
         for (qthread_worker_id_t w = 0; w < qlib->nworkerspershep; w++) {
-          sum += (wkrs[w].current != NULL);
+          sum += (atomic_load_explicit(&wkrs[w].current,
+                                       memory_order_relaxed) != NULL);
         }
       }
       return sum;
@@ -1351,7 +1353,8 @@ qthread_readstate(const enum introspective_state type) { /*{{{ */
       for (qthread_shepherd_id_t s = 0; s < qlib->nshepherds; s++) {
         qthread_worker_t const *wkrs = sheps[s].workers;
         for (qthread_worker_id_t w = 0; w < qlib->nworkerspershep; w++) {
-          count += (wkrs[w].current != NULL);
+          count += (atomic_load_explicit(&wkrs[w].current,
+                                         memory_order_relaxed) != NULL);
         }
       }
       return count;