Skip to content

Commit 3ec21c0

Browse files
insertinterestingnamehereinsertinterestingnamehere
authored
Merge pull request #304 from insertinterestingnamehere/tsan
Thread Sanitizer Fixes
2 parents 2345ecf + e3e1f8f commit 3ec21c0

File tree

9 files changed

+132
-74
lines changed

9 files changed

+132
-74
lines changed

‎.circleci/config.yml‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ jobs:
2727
make -j2
2828
make tests -j2
2929
- run:
30-
command: timeout --foreground -k 10s 2m make check
30+
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 )
3131
no_output_timeout: 180s
3232

3333
arm_clang:
@@ -58,7 +58,7 @@ jobs:
5858
make -j2
5959
make tests -j2
6060
- run:
61-
command: timeout --foreground -k 10s 2m make check
61+
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 )
6262
no_output_timeout: 180s
6363

6464
arm_sanitizers:
@@ -96,7 +96,10 @@ jobs:
9696
make -j2
9797
make tests -j2
9898
- run:
99-
command: timeout --foreground -k 10s 4m make check
99+
command: |
100+
export QTHREADS_DIR="$(pwd)"
101+
if [[ "<< parameters.sanitizer >>" == "thread" ]]; then cd test/basics; fi
102+
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 )
100103
no_output_timeout: 120s
101104

102105
arm_acfl:
@@ -134,7 +137,7 @@ jobs:
134137
- run:
135138
command: |
136139
export PATH=$PATH:/opt/arm/arm-linux-compiler-24.04_Ubuntu-22.04/bin
137-
timeout --foreground -k 10s 4m make check
140+
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 )
138141
no_output_timeout: 180s
139142

140143
nvc:
@@ -173,7 +176,7 @@ jobs:
173176
make tests -j2
174177
- run:
175178
command: |
176-
timeout --foreground -k 10s 4m make check
179+
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 )
177180
no_output_timeout: 180s
178181

179182
musl:
@@ -197,7 +200,7 @@ jobs:
197200
make -j2
198201
make tests -j2
199202
- run:
200-
command: make check
203+
command: make check || ( cat test/basics/test-suite.log && cat test/features/test-suite.log && cat test/stress/test-suite.log && exit 1 )
201204
no_output_timeout: 180s
202205

203206
workflows:
@@ -238,6 +241,24 @@ workflows:
238241
- scheduler: distrib
239242
topology: hwloc
240243
sanitizer: memory
244+
- scheduler: sherwood
245+
topology: 'no'
246+
sanitizer: thread
247+
- scheduler: sherwood
248+
topology: hwloc
249+
sanitizer: thread
250+
- scheduler: sherwood
251+
topology: binders
252+
sanitizer: thread
253+
- scheduler: distrib
254+
topology: 'no'
255+
sanitizer: thread
256+
- scheduler: distrib
257+
topology: hwloc
258+
sanitizer: thread
259+
- scheduler: distrib
260+
topology: binders
261+
sanitizer: thread
241262
- arm_acfl:
242263
matrix:
243264
parameters:

‎.cirrus.yml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ osx_m1_task:
4545
# commented example for how to get a backtrace from CI usign lldb on OSX:
4646
#echo "settings set target.process.stop-on-exec false" > ~/.lldbinit
4747
#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
48-
gtimeout --foreground 3m make check
48+
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 )
4949
5050
freebsd_task:
5151
freebsd_instance:
@@ -68,7 +68,7 @@ freebsd_task:
6868
make -j$CIRRUS_CPU
6969
make tests -j$CIRRUS_CPU
7070
test_script: |
71-
gtimeout --foreground -k 10s 2m make check
71+
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 )
7272
7373
arm_linux_task:
7474
arm_container:
@@ -112,7 +112,7 @@ arm_linux_task:
112112
make -j$CIRRUS_CPU
113113
make tests -j$CIRRUS_CPU
114114
test_script: |
115-
timeout --foreground -k 10s 2m make check
115+
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 )
116116
117117
arm_linux_clang_task:
118118
arm_container:
@@ -168,5 +168,5 @@ arm_linux_clang_task:
168168
make -j$CIRRUS_CPU
169169
make tests -j$CIRRUS_CPU
170170
test_script: |
171-
timeout --foreground -k 10s 2m make check
171+
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 )
172172

‎.github/workflows/CI.yml‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ jobs:
3535
make -j2
3636
make tests -j2
3737
- name: make check
38-
run: timeout -k 10s --foreground 3m make check
38+
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 )
3939
timeout-minutes: 4
4040

4141
linux-clang:
@@ -77,7 +77,7 @@ jobs:
7777
make -j2
7878
make tests -j2
7979
- name: make check
80-
run: timeout -k 10s --foreground 6m make check
80+
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 )
8181
timeout-minutes: 7
8282

8383
linux-icx:
@@ -116,7 +116,7 @@ jobs:
116116
- name: make check
117117
run: |
118118
source /opt/intel/oneapi/setvars.sh
119-
timeout -k 10s --foreground 6m make check
119+
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 )
120120
timeout-minutes: 7
121121

122122
linux-icc:
@@ -157,7 +157,7 @@ jobs:
157157
- name: make check
158158
run: |
159159
source /opt/intel/oneapi/setvars.sh
160-
timeout -k 10s --foreground 6m make check
160+
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 )
161161
timeout-minutes: 7
162162

163163
linux-aocc:
@@ -192,7 +192,7 @@ jobs:
192192
make tests -j2
193193
- name: make check
194194
run: |
195-
timeout -k 10s --foreground 6m make check
195+
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 )
196196
timeout-minutes: 7
197197

198198
mac:
@@ -228,7 +228,7 @@ jobs:
228228
# commented example for how to get a backtrace from CI usign lldb on OSX:
229229
#echo "settings set target.process.stop-on-exec false" > ~/.lldbinit
230230
#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
231-
gtimeout -k 10s --foreground 8m make check
231+
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 )
232232
timeout-minutes: 9
233233

234234
sanitizers:
@@ -245,6 +245,14 @@ jobs:
245245
topology: hwloc
246246
- sanitizer: memory
247247
topology: binders
248+
- sanitizer: thread
249+
scheduler: sherwood
250+
- sanitizer: thread
251+
scheduler: distrib
252+
- sanitizer: thread
253+
topology: hwloc
254+
- sanitizer: thread
255+
topology: binders
248256
env:
249257
CC: clang-19
250258
CXX: clang++-19
@@ -276,7 +284,10 @@ jobs:
276284
make -j2
277285
make tests -j2
278286
- name: make check
279-
run: timeout -k 10s --foreground 8m make check
287+
run: |
288+
export QTHREADS_DIR="$(pwd)"
289+
if [[ "${{ matrix.sanitizer }}" == "thread" ]]; then cd test/basics; fi
290+
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 )
280291
timeout-minutes: 9
281292

282293
linux-thorough:
@@ -319,7 +330,7 @@ jobs:
319330
make -j2
320331
make tests -j2
321332
- name: make check
322-
run: timeout -k 10s --foreground 6m make check
333+
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 )
323334
timeout-minutes: 7
324335

325336
clang-format:

‎include/qt_queue.h‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,24 +9,25 @@
99
*/
1010

1111
typedef struct qthread_queue_node_s {
12-
struct qthread_queue_node_s *next;
13-
qthread_t *thread;
12+
struct qthread_queue_node_s *_Atomic next;
13+
qthread_t *_Atomic thread;
1414
} qthread_queue_node_t;
1515

1616
typedef struct qthread_queue_NEMESIS_s {
1717
/* The First Cacheline */
18-
void *head;
19-
void *tail;
20-
uint8_t pad1[CACHELINE_WIDTH - (2 * sizeof(void *))];
18+
void *_Atomic head;
19+
uint8_t pad1[CACHELINE_WIDTH - sizeof(void *)];
20+
void *_Atomic tail;
21+
uint8_t pad2[CACHELINE_WIDTH - sizeof(void *)];
2122
/* The Second Cacheline */
22-
aligned_t length;
23+
_Atomic aligned_t length;
2324
void *shadow_head;
24-
uint8_t pad2[CACHELINE_WIDTH - sizeof(void *) - sizeof(aligned_t)];
25+
uint8_t pad3[CACHELINE_WIDTH - sizeof(void *) - sizeof(aligned_t)];
2526
} qthread_queue_NEMESIS_t;
2627

2728
typedef struct qthread_queue_nosync_s {
28-
qthread_queue_node_t *head;
29-
qthread_queue_node_t *tail;
29+
qthread_queue_node_t *_Atomic head;
30+
qthread_queue_node_t *_Atomic tail;
3031
} qthread_queue_nosync_t;
3132

3233
typedef struct qthread_queue_capped_s {

‎include/qt_shepherd_innards.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ struct qthread_worker_s {
3636
qthread_shepherd_t *shepherd;
3737
struct qthread_s **nostealbuffer;
3838
struct qthread_s **stealbuffer;
39-
qthread_t *current;
39+
qthread_t *_Atomic current;
4040
qthread_worker_id_t unique_id;
4141
qthread_worker_id_t worker_id;
4242
qthread_worker_id_t packed_worker_id;

‎src/qthread.c‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ static void *qthread_master(void *arg) {
334334
qt_threadqueue_t *threadqueue;
335335
qt_threadqueue_private_t *localqueue = NULL;
336336
qthread_t *t;
337-
qthread_t **current;
337+
qthread_t *_Atomic *current;
338338
int done = 0;
339339

340340
assert(me != NULL);
@@ -444,8 +444,9 @@ static void *qthread_master(void *arg) {
444444
#endif
445445
qthread_exec(t, &my_context);
446446

447-
t = *current; // necessary for direct-swap sanity
448-
*current = NULL; // neessary for "queue sanity"
447+
t = *current; // necessary for direct-swap sanity
448+
atomic_store_explicit(
449+
current, NULL, memory_order_relaxed); // neessary for "queue sanity"
449450

450451
/* now clean up, based on the thread's state */
451452
switch (atomic_load_explicit(&t->thread_state, memory_order_relaxed)) {
@@ -1340,7 +1341,8 @@ qthread_readstate(const enum introspective_state type) { /*{{{ */
13401341
sum += qt_threadqueue_advisory_queuelen(sheps[s].ready);
13411342
qthread_worker_t const *wkrs = sheps[s].workers;
13421343
for (qthread_worker_id_t w = 0; w < qlib->nworkerspershep; w++) {
1343-
sum += (wkrs[w].current != NULL);
1344+
sum += (atomic_load_explicit(&wkrs[w].current,
1345+
memory_order_relaxed) != NULL);
13441346
}
13451347
}
13461348
return sum;
@@ -1351,7 +1353,8 @@ qthread_readstate(const enum introspective_state type) { /*{{{ */
13511353
for (qthread_shepherd_id_t s = 0; s < qlib->nshepherds; s++) {
13521354
qthread_worker_t const *wkrs = sheps[s].workers;
13531355
for (qthread_worker_id_t w = 0; w < qlib->nworkerspershep; w++) {
1354-
count += (wkrs[w].current != NULL);
1356+
count += (atomic_load_explicit(&wkrs[w].current,
1357+
memory_order_relaxed) != NULL);
13551358
}
13561359
}
13571360
return count;

0 commit comments

Comments
 (0)