Merge branch 'jk/lsan-race-with-barrier'

CI jobs that run threaded programs under LSan has been giving false
positives from time to time, which has been worked around.

* 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

6 files changed, 40 insertions, 3 deletions

diff --git a/Makefile b/Makefile
index 97e8385b66..2c6dad8a75 100644
--- a/Makefile
+++ b/Makefile
@@ -141,6 +141,10 @@ include shared.mak
 #
 # Define NO_PTHREADS if you do not have or do not want to use Pthreads.
 #
+# Define THREAD_BARRIER_PTHREAD if your system has pthread_barrier_t. Barrier
+# support is optional and is only helpful when building with SANITIZE=leak, as
+# it is used to eliminate some races in the leak-checker.
+#
 # Define NO_PREAD if you have a problem with pread() system call (e.g.
 # cygwin1.dll before v1.5.22).
 #
@@ -2079,6 +2083,9 @@ ifdef NO_PTHREADS
 else
 	BASIC_CFLAGS += $(PTHREAD_CFLAGS)
 	EXTLIBS += $(PTHREAD_LIBS)
+	ifdef THREAD_BARRIER_PTHREAD
+		BASIC_CFLAGS += -DTHREAD_BARRIER_PTHREAD
+	endif
 endif
 
 ifdef HAVE_PATHS_H
diff --git a/builtin/grep.c b/builtin/grep.c
index d00ee76f24..61b2c27490 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -101,6 +101,9 @@ static pthread_cond_t cond_write;
 /* Signalled when we are finished with everything. */
 static pthread_cond_t cond_result;
 
+/* Synchronize the start of all threads */
+static maybe_thread_barrier_t start_barrier;
+
 static int skip_first_line;
 
 static void add_work(struct grep_opt *opt, struct grep_source *gs)
@@ -198,6 +201,8 @@ static void *run(void *arg)
 	int hit = 0;
 	struct grep_opt *opt = arg;
 
+	maybe_thread_barrier_wait(&start_barrier);
+
 	while (1) {
 		struct work_item *w = get_work();
 		if (!w)
@@ -229,6 +234,7 @@ static void start_threads(struct grep_opt *opt)
 	pthread_cond_init(&cond_add, NULL);
 	pthread_cond_init(&cond_write, NULL);
 	pthread_cond_init(&cond_result, NULL);
+	maybe_thread_barrier_init(&start_barrier, NULL, num_threads + 1);
 	grep_use_locks = 1;
 	enable_obj_read_lock();
 
@@ -248,6 +254,7 @@ static void start_threads(struct grep_opt *opt)
 			die(_("grep: failed to create thread: %s"),
 			    strerror(err));
 	}
+	maybe_thread_barrier_wait(&start_barrier);
 }
 
 static int wait_all(void)
@@ -284,6 +291,7 @@ static int wait_all(void)
 	pthread_cond_destroy(&cond_add);
 	pthread_cond_destroy(&cond_write);
 	pthread_cond_destroy(&cond_result);
+	maybe_thread_barrier_destroy(&start_barrier);
 	grep_use_locks = 0;
 	disable_obj_read_lock();
 
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index d773809c4c..27b120f26c 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -185,6 +185,8 @@ static pthread_mutex_t deepest_delta_mutex;
 
 static pthread_key_t key;
 
+static maybe_thread_barrier_t start_barrier;
+
 static inline void lock_mutex(pthread_mutex_t *mutex)
 {
 	if (threads_active)
@@ -209,6 +211,7 @@ static void init_thread(void)
 	if (show_stat)
 		pthread_mutex_init(&deepest_delta_mutex, NULL);
 	pthread_key_create(&key, NULL);
+	maybe_thread_barrier_init(&start_barrier, NULL, nr_threads);
 	CALLOC_ARRAY(thread_data, nr_threads);
 	for (i = 0; i < nr_threads; i++) {
 		thread_data[i].pack_fd = xopen(curr_pack, O_RDONLY);
@@ -231,6 +234,7 @@ static void cleanup_thread(void)
 	for (i = 0; i < nr_threads; i++)
 		close(thread_data[i].pack_fd);
 	pthread_key_delete(key);
+	maybe_thread_barrier_destroy(&start_barrier);
 	free(thread_data);
 }
 
@@ -1100,6 +1104,8 @@ static int compare_ref_delta_entry(const void *a, const void *b)
 
 static void *threaded_second_pass(void *data)
 {
+	if (threads_active)
+		maybe_thread_barrier_wait(&start_barrier);
 	if (data)
 		set_thread_data(data);
 	for (;;) {
@@ -1336,7 +1342,6 @@ static void resolve_deltas(struct pack_idx_option *opts)
 	base_cache_limit = opts->delta_base_cache_limit * nr_threads;
 	if (nr_threads > 1 || getenv("GIT_FORCE_THREADS")) {
 		init_thread();
-		work_lock();
 		for (i = 0; i < nr_threads; i++) {
 			int ret = pthread_create(&thread_data[i].thread, NULL,
 						 threaded_second_pass, thread_data + i);
@@ -1344,7 +1349,6 @@ static void resolve_deltas(struct pack_idx_option *opts)
 				die(_("unable to create thread: %s"),
 				    strerror(ret));
 		}
-		work_unlock();
 		for (i = 0; i < nr_threads; i++)
 			pthread_join(thread_data[i].thread, NULL);
 		cleanup_thread();
diff --git a/ci/lib.sh b/ci/lib.sh
index 8885ee3c3f..6a1267fbcb 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -385,6 +385,7 @@ linux-musl)
 	;;
 linux-leaks|linux-reftable-leaks)
 	export SANITIZE=leak
+	export THREAD_BARRIER_PTHREAD=1
 	;;
 linux-asan-ubsan)
 	export SANITIZE=address,undefined
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1a67adb207..96f2dfb69d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -331,7 +331,7 @@ TEST_RESULTS_BASE="$TEST_RESULTS_DIR/$TEST_NAME$TEST_STRESS_JOB_SFX"
 TEST_RESULTS_SAN_FILE_PFX=trace
 TEST_RESULTS_SAN_DIR_SFX=leak
 TEST_RESULTS_SAN_FILE=
-TEST_RESULTS_SAN_DIR="$TEST_RESULTS_DIR/$TEST_NAME.$TEST_RESULTS_SAN_DIR_SFX"
+TEST_RESULTS_SAN_DIR="$TEST_RESULTS_BASE.$TEST_RESULTS_SAN_DIR_SFX"
 TRASH_DIRECTORY="trash directory.$TEST_NAME$TEST_STRESS_JOB_SFX"
 test -n "$root" && TRASH_DIRECTORY="$root/$TRASH_DIRECTORY"
 case "$TRASH_DIRECTORY" in
diff --git a/thread-utils.h b/thread-utils.h
index 4961487ed9..3df5be9916 100644
--- a/thread-utils.h
+++ b/thread-utils.h
@@ -53,5 +53,22 @@ int dummy_pthread_init(void *);
 int online_cpus(void);
 int init_recursive_mutex(pthread_mutex_t*);
 
+#ifdef THREAD_BARRIER_PTHREAD
+#define maybe_thread_barrier_t pthread_barrier_t
+#define maybe_thread_barrier_init pthread_barrier_init
+#define maybe_thread_barrier_wait pthread_barrier_wait
+#define maybe_thread_barrier_destroy pthread_barrier_destroy
+#else
+#define maybe_thread_barrier_t int
+static inline int maybe_thread_barrier_init(maybe_thread_barrier_t *b UNUSED,
+					    void *attr UNUSED,
+					    unsigned nr UNUSED)
+{
+	errno = ENOSYS;
+	return -1;
+}
+#define maybe_thread_barrier_wait(barrier)
+#define maybe_thread_barrier_destroy(barrier)
+#endif
 
 #endif /* THREAD_COMPAT_H */