Skip to content

Commit effbef2

Browse files
committed
Merge branch 'jk/lsan-race-ignore-false-positive'
CI jobs that run threaded programs under LSan has been giving false positives from time to time, which has been worked around. This is an alternative to the jk/lsan-race-with-barrier topic with much smaller change to the production code. * jk/lsan-race-ignore-false-positive: test-lib: ignore leaks in the sanitizer's thread code test-lib: check leak logs for presence of DEDUP_TOKEN test-lib: simplify leak-log checking test-lib: rely on logs to detect leaks Revert barrier-based LSan threading race workaround
2 parents d062ccf + b119a68 commit effbef2

File tree

6 files changed

+10
-52
lines changed

6 files changed

+10
-52
lines changed

Makefile

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,6 @@ include shared.mak
141141
#
142142
# Define NO_PTHREADS if you do not have or do not want to use Pthreads.
143143
#
144-
# Define THREAD_BARRIER_PTHREAD if your system has pthread_barrier_t. Barrier
145-
# support is optional and is only helpful when building with SANITIZE=leak, as
146-
# it is used to eliminate some races in the leak-checker.
147-
#
148144
# Define NO_PREAD if you have a problem with pread() system call (e.g.
149145
# cygwin1.dll before v1.5.22).
150146
#
@@ -2083,9 +2079,6 @@ ifdef NO_PTHREADS
20832079
else
20842080
BASIC_CFLAGS += $(PTHREAD_CFLAGS)
20852081
EXTLIBS += $(PTHREAD_LIBS)
2086-
ifdef THREAD_BARRIER_PTHREAD
2087-
BASIC_CFLAGS += -DTHREAD_BARRIER_PTHREAD
2088-
endif
20892082
endif
20902083

20912084
ifdef HAVE_PATHS_H

builtin/grep.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,6 @@ static pthread_cond_t cond_write;
101101
/* Signalled when we are finished with everything. */
102102
static pthread_cond_t cond_result;
103103

104-
/* Synchronize the start of all threads */
105-
static maybe_thread_barrier_t start_barrier;
106-
107104
static int skip_first_line;
108105

109106
static void add_work(struct grep_opt *opt, struct grep_source *gs)
@@ -201,8 +198,6 @@ static void *run(void *arg)
201198
int hit = 0;
202199
struct grep_opt *opt = arg;
203200

204-
maybe_thread_barrier_wait(&start_barrier);
205-
206201
while (1) {
207202
struct work_item *w = get_work();
208203
if (!w)
@@ -234,7 +229,6 @@ static void start_threads(struct grep_opt *opt)
234229
pthread_cond_init(&cond_add, NULL);
235230
pthread_cond_init(&cond_write, NULL);
236231
pthread_cond_init(&cond_result, NULL);
237-
maybe_thread_barrier_init(&start_barrier, NULL, num_threads + 1);
238232
grep_use_locks = 1;
239233
enable_obj_read_lock();
240234

@@ -254,7 +248,6 @@ static void start_threads(struct grep_opt *opt)
254248
die(_("grep: failed to create thread: %s"),
255249
strerror(err));
256250
}
257-
maybe_thread_barrier_wait(&start_barrier);
258251
}
259252

260253
static int wait_all(void)
@@ -291,7 +284,6 @@ static int wait_all(void)
291284
pthread_cond_destroy(&cond_add);
292285
pthread_cond_destroy(&cond_write);
293286
pthread_cond_destroy(&cond_result);
294-
maybe_thread_barrier_destroy(&start_barrier);
295287
grep_use_locks = 0;
296288
disable_obj_read_lock();
297289

builtin/index-pack.c

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,6 @@ static pthread_mutex_t deepest_delta_mutex;
185185

186186
static pthread_key_t key;
187187

188-
static maybe_thread_barrier_t start_barrier;
189-
190188
static inline void lock_mutex(pthread_mutex_t *mutex)
191189
{
192190
if (threads_active)
@@ -211,7 +209,6 @@ static void init_thread(void)
211209
if (show_stat)
212210
pthread_mutex_init(&deepest_delta_mutex, NULL);
213211
pthread_key_create(&key, NULL);
214-
maybe_thread_barrier_init(&start_barrier, NULL, nr_threads);
215212
CALLOC_ARRAY(thread_data, nr_threads);
216213
for (i = 0; i < nr_threads; i++) {
217214
thread_data[i].pack_fd = xopen(curr_pack, O_RDONLY);
@@ -234,7 +231,6 @@ static void cleanup_thread(void)
234231
for (i = 0; i < nr_threads; i++)
235232
close(thread_data[i].pack_fd);
236233
pthread_key_delete(key);
237-
maybe_thread_barrier_destroy(&start_barrier);
238234
free(thread_data);
239235
}
240236

@@ -1104,8 +1100,6 @@ static int compare_ref_delta_entry(const void *a, const void *b)
11041100

11051101
static void *threaded_second_pass(void *data)
11061102
{
1107-
if (threads_active)
1108-
maybe_thread_barrier_wait(&start_barrier);
11091103
if (data)
11101104
set_thread_data(data);
11111105
for (;;) {

ci/lib.sh

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,6 @@ linux-musl)
385385
;;
386386
linux-leaks|linux-reftable-leaks)
387387
export SANITIZE=leak
388-
export THREAD_BARRIER_PTHREAD=1
389388
;;
390389
linux-asan-ubsan)
391390
export SANITIZE=address,undefined

t/test-lib.sh

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ prepend_var ASAN_OPTIONS : detect_leaks=0
8080
export ASAN_OPTIONS
8181

8282
prepend_var LSAN_OPTIONS : $GIT_SAN_OPTIONS
83+
prepend_var LSAN_OPTIONS : exitcode=0
8384
prepend_var LSAN_OPTIONS : fast_unwind_on_malloc=0
8485
export LSAN_OPTIONS
8586

@@ -339,17 +340,6 @@ case "$TRASH_DIRECTORY" in
339340
*) TRASH_DIRECTORY="$TEST_OUTPUT_DIRECTORY/$TRASH_DIRECTORY" ;;
340341
esac
341342

342-
# Utility functions using $TEST_RESULTS_* variables
343-
nr_san_dir_leaks_ () {
344-
# stderr piped to /dev/null because the directory may have
345-
# been "rmdir"'d already.
346-
find "$TEST_RESULTS_SAN_DIR" \
347-
-type f \
348-
-name "$TEST_RESULTS_SAN_FILE_PFX.*" 2>/dev/null |
349-
xargs grep -lv "Unable to get registers from thread" |
350-
wc -l
351-
}
352-
353343
# If --stress was passed, run this test repeatedly in several parallel loops.
354344
if test "$GIT_TEST_STRESS_STARTED" = "done"
355345
then
@@ -1180,8 +1170,15 @@ test_atexit_handler () {
11801170
}
11811171

11821172
check_test_results_san_file_empty_ () {
1183-
test -z "$TEST_RESULTS_SAN_FILE" ||
1184-
test "$(nr_san_dir_leaks_)" = 0
1173+
test -z "$TEST_RESULTS_SAN_FILE" && return 0
1174+
1175+
# stderr piped to /dev/null because the directory may have
1176+
# been "rmdir"'d already.
1177+
! find "$TEST_RESULTS_SAN_DIR" \
1178+
-type f \
1179+
-name "$TEST_RESULTS_SAN_FILE_PFX.*" 2>/dev/null |
1180+
xargs grep ^DEDUP_TOKEN |
1181+
grep -qv sanitizer::GetThreadStackTopAndBottom
11851182
}
11861183

11871184
check_test_results_san_file_ () {

thread-utils.h

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -53,22 +53,5 @@ int dummy_pthread_init(void *);
5353
int online_cpus(void);
5454
int init_recursive_mutex(pthread_mutex_t*);
5555

56-
#ifdef THREAD_BARRIER_PTHREAD
57-
#define maybe_thread_barrier_t pthread_barrier_t
58-
#define maybe_thread_barrier_init pthread_barrier_init
59-
#define maybe_thread_barrier_wait pthread_barrier_wait
60-
#define maybe_thread_barrier_destroy pthread_barrier_destroy
61-
#else
62-
#define maybe_thread_barrier_t int
63-
static inline int maybe_thread_barrier_init(maybe_thread_barrier_t *b UNUSED,
64-
void *attr UNUSED,
65-
unsigned nr UNUSED)
66-
{
67-
errno = ENOSYS;
68-
return -1;
69-
}
70-
#define maybe_thread_barrier_wait(barrier)
71-
#define maybe_thread_barrier_destroy(barrier)
72-
#endif
7356

7457
#endif /* THREAD_COMPAT_H */

0 commit comments

Comments
 (0)