Skip to content

Commit 3fc0e14

Browse files
committed
Merge branch 'jk/lsan-race-with-barrier' into next
* jk/lsan-race-with-barrier: grep: work around LSan threading race with barrier index-pack: work around LSan threading race with barrier thread-utils: introduce optional barrier type Revert "index-pack: spawn threads atomically" test-lib: use individual lsan dir for --stress runs
2 parents e083ea3 + 7a8d9ef commit 3fc0e14

File tree

6 files changed

+40
-3
lines changed

6 files changed

+40
-3
lines changed

Makefile

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,10 @@ 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+
#
144148
# Define NO_PREAD if you have a problem with pread() system call (e.g.
145149
# cygwin1.dll before v1.5.22).
146150
#
@@ -2079,6 +2083,9 @@ ifdef NO_PTHREADS
20792083
else
20802084
BASIC_CFLAGS += $(PTHREAD_CFLAGS)
20812085
EXTLIBS += $(PTHREAD_LIBS)
2086+
ifdef THREAD_BARRIER_PTHREAD
2087+
BASIC_CFLAGS += -DTHREAD_BARRIER_PTHREAD
2088+
endif
20822089
endif
20832090

20842091
ifdef HAVE_PATHS_H

builtin/grep.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@ 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+
104107
static int skip_first_line;
105108

106109
static void add_work(struct grep_opt *opt, struct grep_source *gs)
@@ -198,6 +201,8 @@ static void *run(void *arg)
198201
int hit = 0;
199202
struct grep_opt *opt = arg;
200203

204+
maybe_thread_barrier_wait(&start_barrier);
205+
201206
while (1) {
202207
struct work_item *w = get_work();
203208
if (!w)
@@ -229,6 +234,7 @@ static void start_threads(struct grep_opt *opt)
229234
pthread_cond_init(&cond_add, NULL);
230235
pthread_cond_init(&cond_write, NULL);
231236
pthread_cond_init(&cond_result, NULL);
237+
maybe_thread_barrier_init(&start_barrier, NULL, num_threads + 1);
232238
grep_use_locks = 1;
233239
enable_obj_read_lock();
234240

@@ -248,6 +254,7 @@ static void start_threads(struct grep_opt *opt)
248254
die(_("grep: failed to create thread: %s"),
249255
strerror(err));
250256
}
257+
maybe_thread_barrier_wait(&start_barrier);
251258
}
252259

253260
static int wait_all(void)
@@ -284,6 +291,7 @@ static int wait_all(void)
284291
pthread_cond_destroy(&cond_add);
285292
pthread_cond_destroy(&cond_write);
286293
pthread_cond_destroy(&cond_result);
294+
maybe_thread_barrier_destroy(&start_barrier);
287295
grep_use_locks = 0;
288296
disable_obj_read_lock();
289297

builtin/index-pack.c

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

186186
static pthread_key_t key;
187187

188+
static maybe_thread_barrier_t start_barrier;
189+
188190
static inline void lock_mutex(pthread_mutex_t *mutex)
189191
{
190192
if (threads_active)
@@ -209,6 +211,7 @@ static void init_thread(void)
209211
if (show_stat)
210212
pthread_mutex_init(&deepest_delta_mutex, NULL);
211213
pthread_key_create(&key, NULL);
214+
maybe_thread_barrier_init(&start_barrier, NULL, nr_threads);
212215
CALLOC_ARRAY(thread_data, nr_threads);
213216
for (i = 0; i < nr_threads; i++) {
214217
thread_data[i].pack_fd = xopen(curr_pack, O_RDONLY);
@@ -231,6 +234,7 @@ static void cleanup_thread(void)
231234
for (i = 0; i < nr_threads; i++)
232235
close(thread_data[i].pack_fd);
233236
pthread_key_delete(key);
237+
maybe_thread_barrier_destroy(&start_barrier);
234238
free(thread_data);
235239
}
236240

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

11011105
static void *threaded_second_pass(void *data)
11021106
{
1107+
if (threads_active)
1108+
maybe_thread_barrier_wait(&start_barrier);
11031109
if (data)
11041110
set_thread_data(data);
11051111
for (;;) {
@@ -1336,15 +1342,13 @@ static void resolve_deltas(struct pack_idx_option *opts)
13361342
base_cache_limit = opts->delta_base_cache_limit * nr_threads;
13371343
if (nr_threads > 1 || getenv("GIT_FORCE_THREADS")) {
13381344
init_thread();
1339-
work_lock();
13401345
for (i = 0; i < nr_threads; i++) {
13411346
int ret = pthread_create(&thread_data[i].thread, NULL,
13421347
threaded_second_pass, thread_data + i);
13431348
if (ret)
13441349
die(_("unable to create thread: %s"),
13451350
strerror(ret));
13461351
}
1347-
work_unlock();
13481352
for (i = 0; i < nr_threads; i++)
13491353
pthread_join(thread_data[i].thread, NULL);
13501354
cleanup_thread();

ci/lib.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,7 @@ linux-musl)
385385
;;
386386
linux-leaks|linux-reftable-leaks)
387387
export SANITIZE=leak
388+
export THREAD_BARRIER_PTHREAD=1
388389
;;
389390
linux-asan-ubsan)
390391
export SANITIZE=address,undefined

t/test-lib.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ TEST_RESULTS_BASE="$TEST_RESULTS_DIR/$TEST_NAME$TEST_STRESS_JOB_SFX"
331331
TEST_RESULTS_SAN_FILE_PFX=trace
332332
TEST_RESULTS_SAN_DIR_SFX=leak
333333
TEST_RESULTS_SAN_FILE=
334-
TEST_RESULTS_SAN_DIR="$TEST_RESULTS_DIR/$TEST_NAME.$TEST_RESULTS_SAN_DIR_SFX"
334+
TEST_RESULTS_SAN_DIR="$TEST_RESULTS_BASE.$TEST_RESULTS_SAN_DIR_SFX"
335335
TRASH_DIRECTORY="trash directory.$TEST_NAME$TEST_STRESS_JOB_SFX"
336336
test -n "$root" && TRASH_DIRECTORY="$root/$TRASH_DIRECTORY"
337337
case "$TRASH_DIRECTORY" in

thread-utils.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,5 +53,22 @@ 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
5673

5774
#endif /* THREAD_COMPAT_H */

0 commit comments

Comments
 (0)