Skip to content

Commit 7a8d9ef

Browse files
peffgitster
authored andcommitted
grep: work around LSan threading race with barrier
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]>
1 parent 526c0a8 commit 7a8d9ef

File tree

1 file changed

+8
-0
lines changed

1 file changed

+8
-0
lines changed

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

0 commit comments

Comments
 (0)