Skip to content

Commit

Permalink
grep: work around LSan threading race with barrier
Browse files Browse the repository at this point in the history
There's a race with LSan when spawning threads and one of the threads
calls die(). We worked around one such problem with index-pack in the
previous commit, but it exists in git-grep, too. You can see it with:

  make SANITIZE=leak THREAD_BARRIER_PTHREAD=YesOnLinux
  cd t
  ./t0003-attributes.sh --stress

which fails pretty quickly with:

  ==git==4096424==ERROR: LeakSanitizer: detected memory leaks

  Direct leak of 32 byte(s) in 1 object(s) allocated from:
      #0 0x7f906de14556 in realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98
      #1 0x7f906dc9d2c1 in __pthread_getattr_np nptl/pthread_getattr_np.c:180
      #2 0x7f906de2500d in __sanitizer::GetThreadStackTopAndBottom(bool, unsigned long*, unsigned long*) ../../../../src/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cpp:150
      #3 0x7f906de25187 in __sanitizer::GetThreadStackAndTls(bool, unsigned long*, unsigned long*, unsigned long*, unsigned long*) ../../../../src/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cpp:614
      #4 0x7f906de17d18 in __lsan::ThreadStart(unsigned int, unsigned long long, __sanitizer::ThreadType) ../../../../src/libsanitizer/lsan/lsan_posix.cpp:53
      #5 0x7f906de143a9 in ThreadStartFunc<false> ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:431
      #6 0x7f906dc9bf51 in start_thread nptl/pthread_create.c:447
      #7 0x7f906dd1a677 in __clone3 ../sysdeps/unix/sysv/linux/x86_64/clone3.S:78

As with the previous commit, we can fix this by inserting a barrier that
makes sure all threads have finished their setup before continuing. But
there's one twist in this case: the thread which calls die() is not one
of the worker threads, but the main thread itself!

So we need the main thread to wait in the barrier, too, until all
threads have gotten to it. And thus we initialize the barrier for
num_threads+1, to account for all of the worker threads plus the main
one.

If we then test as above, t0003 should run indefinitely.

Signed-off-by: Jeff King <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
  • Loading branch information
peff authored and gitster committed Dec 30, 2024
1 parent 526c0a8 commit 7a8d9ef
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions builtin/grep.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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();

Expand All @@ -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)
Expand Down Expand Up @@ -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();

Expand Down

0 comments on commit 7a8d9ef

Please sign in to comment.