diff mbox series

[4/5] index-pack: work around LSan threading race with barrier

Message ID 20241230042938.GD113400@coredump.intra.peff.net (mailing list archive)
State Accepted
Commit 526c0a851b14d1bbec4b8d31a23d93ca0eb82637
Headers show
Series fixing thread races in linux-leaks CI | expand

Commit Message

Jeff King Dec. 30, 2024, 4:29 a.m. UTC
We sometimes get false positives from our linux-leaks CI job because of
a race in LSan itself. The problem is that one thread is still
initializing its stack in LSan's code (and allocating memory to do so)
while anothe thread calls die(), taking down the whole process and
triggering a leak check.

The problem is described in more detail in 993d38a066 (index-pack: spawn
threads atomically, 2024-01-05), which tried to fix it by pausing worker
threads until all calls to pthread_create() had completed. But that's
not enough to fix the problem, because the LSan setup code runs in the
threads themselves. So even though pthread_create() has returned, we
have no idea if all threads actually finished their setup before letting
any of them do real work.

We can fix that by using a barrier inside the threads themselves,
waiting for all of them to hit the start of their main function before
any of them proceed.

You can test for the race by running:

  make SANITIZE=leak THREAD_BARRIER_PTHREAD=YesOnLinux
  cd t
  ./t5309-pack-delta-cycles.sh --stress

which fails quickly before this patch, and should run indefinitely
without it.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/index-pack.c | 6 ++++++
 1 file changed, 6 insertions(+)
diff mbox series

Patch

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 0b62b2589f..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 (;;) {