From cdf055376907cfa7837e576db219a1a7617df276 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 25 Oct 2011 12:25:20 -0500 Subject: [PATCH 1/3] git grep: be careful to use mutexes only when they are initialized Rather nasty things happen when a mutex is not initialized but locked nevertheless. Now, when we're not running in a threaded manner, the mutex is not initialized, which is correct. But then we went and used the mutex anyway, which -- at least on Windows -- leads to a hard crash (ordinarily it would be called a segmentation fault, but in Windows speak it is an access violation). This problem was identified by our faithful tests when run in the msysGit environment. To avoid having to wrap the line due to the 80 column limit, we use the name "WHEN_THREADED" instead of "IF_USE_THREADS" because it is one character shorter. Which is all we need in this case. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/grep.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/builtin/grep.c b/builtin/grep.c index 7d0779f6cf..88b0c80137 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -77,10 +77,11 @@ static pthread_mutex_t grep_mutex; /* Used to serialize calls to read_sha1_file. */ static pthread_mutex_t read_sha1_mutex; -#define grep_lock() pthread_mutex_lock(&grep_mutex) -#define grep_unlock() pthread_mutex_unlock(&grep_mutex) -#define read_sha1_lock() pthread_mutex_lock(&read_sha1_mutex) -#define read_sha1_unlock() pthread_mutex_unlock(&read_sha1_mutex) +#define WHEN_THREADED(x) do { if (use_threads) (x); } while (0) +#define grep_lock() WHEN_THREADED(pthread_mutex_lock(&grep_mutex)) +#define grep_unlock() WHEN_THREADED(pthread_mutex_unlock(&grep_mutex)) +#define read_sha1_lock() WHEN_THREADED(pthread_mutex_lock(&read_sha1_mutex)) +#define read_sha1_unlock() WHEN_THREADED(pthread_mutex_unlock(&read_sha1_mutex)) /* Signalled when a new work_item is added to todo. */ static pthread_cond_t cond_add; From 1487a12ba228c6003f503366f40840eee9887179 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 26 Oct 2011 11:45:15 -0700 Subject: [PATCH 2/3] builtin/grep: make lock/unlock into static inline functions Signed-off-by: Junio C Hamano --- builtin/grep.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/builtin/grep.c b/builtin/grep.c index 88b0c80137..3ddfae4e79 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -74,14 +74,32 @@ static int all_work_added; /* This lock protects all the variables above. */ static pthread_mutex_t grep_mutex; +static inline void grep_lock(void) +{ + if (use_threads) + pthread_mutex_lock(&grep_mutex); +} + +static inline void grep_unlock(void) +{ + if (use_threads) + pthread_mutex_unlock(&grep_mutex); +} + /* Used to serialize calls to read_sha1_file. */ static pthread_mutex_t read_sha1_mutex; -#define WHEN_THREADED(x) do { if (use_threads) (x); } while (0) -#define grep_lock() WHEN_THREADED(pthread_mutex_lock(&grep_mutex)) -#define grep_unlock() WHEN_THREADED(pthread_mutex_unlock(&grep_mutex)) -#define read_sha1_lock() WHEN_THREADED(pthread_mutex_lock(&read_sha1_mutex)) -#define read_sha1_unlock() WHEN_THREADED(pthread_mutex_unlock(&read_sha1_mutex)) +static inline void read_sha1_lock(void) +{ + if (use_threads) + pthread_mutex_lock(&read_sha1_mutex); +} + +static inline void read_sha1_unlock(void) +{ + if (use_threads) + pthread_mutex_unlock(&read_sha1_mutex); +} /* Signalled when a new work_item is added to todo. */ static pthread_cond_t cond_add; From 764161391f2dd3fc12a5bec1385e0ab40dd90fd6 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 26 Oct 2011 12:15:51 -0700 Subject: [PATCH 3/3] builtin/grep: simplify lock_and_read_sha1_file() As read_sha1_lock/unlock have been made aware of use_threads, this caller can be made a lot simpler. Signed-off-by: Junio C Hamano --- builtin/grep.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/builtin/grep.c b/builtin/grep.c index 3ddfae4e79..3d7329d78c 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -373,13 +373,9 @@ static void *lock_and_read_sha1_file(const unsigned char *sha1, enum object_type { void *data; - if (use_threads) { - read_sha1_lock(); - data = read_sha1_file(sha1, type, size); - read_sha1_unlock(); - } else { - data = read_sha1_file(sha1, type, size); - } + read_sha1_lock(); + data = read_sha1_file(sha1, type, size); + read_sha1_unlock(); return data; }