Re: What's cooking in git.git (Apr 2012, #06; Sun, 15)

From

Nguyen Thai Ngoc Duy <pclouds@gmail.com>

Date

Apr 19, 2012, 09:36 UTC

Message-ID

<20120419093608.GA19391@duynguyen-vnpc.dek-tpc.internal>

In-Reply-To

<4F8FB4A0.7090403@viscovery.net>

On Thu, Apr 19, 2012 at 08:45:52AM +0200, Johannes Sixt wrote:

> I don't see any mutual exclusion happening in this chain. Perhaps it is
> not needed, provided that the pread() call in get_data_from_pack is
> atomic. But our git_pread() from compat/pread.c, which we use on Windows,
> is not atomic.

-- 8< --

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 847dbb3..5d539cd 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -102,6 +102,10 @@ static pthread_mutex_t work_mutex;
 #define work_lock()		lock_mutex(&work_mutex)
 #define work_unlock()		unlock_mutex(&work_mutex)
 
+#ifdef WIN32
+static pthread_mutex_t pread_mutex;
+#endif
+
 static pthread_key_t key;
 
 static inline void lock_mutex(pthread_mutex_t *mutex)
@@ -124,6 +128,9 @@ static void init_thread(void)
 	init_recursive_mutex(&read_mutex);
 	pthread_mutex_init(&counter_mutex, NULL);
 	pthread_mutex_init(&work_mutex, NULL);
+#ifdef WIN32
+	pthread_mutex_init(&pread_mutex, NULL);
+#endif
 	pthread_key_create(&key, NULL);
 	thread_data = xcalloc(nr_threads, sizeof(*thread_data));
 	threads_active = 1;
@@ -137,6 +144,9 @@ static void cleanup_thread(void)
 	pthread_mutex_destroy(&read_mutex);
 	pthread_mutex_destroy(&counter_mutex);
 	pthread_mutex_destroy(&work_mutex);
+#ifdef WIN32
+	pthread_mutex_destroy(&pread_mutex);
+#endif
 	pthread_key_delete(key);
 	free(thread_data);
 }
@@ -456,6 +466,20 @@ static void *unpack_raw_entry(struct object_entry *obj, union delta_base *delta_
 	return data;
 }
 
+#ifdef WIN32
+/* pread is reimplemented in compat/pread.c and is not thread-safe */
+static ssize_t pread_threadsafe(int fd, void *buf, size_t count, off_t from)
+{
+	ssize_t n;
+	lock_mutex(&pread_mutex);
+	n = pread(fd, buf, count, from);
+	unlock_mutex(&pread_mutex);
+	return n;
+}
+#else
+#define pread_threadsafe(fd,buf,count,from) pread(fd, buf, count, from)
+#endif
+
 static void *get_data_from_pack(struct object_entry *obj)
 {
 	off_t from = obj[0].idx.offset + obj[0].hdr_size;
@@ -474,7 +498,7 @@ static void *get_data_from_pack(struct object_entry *obj)
 
 	do {
 		ssize_t n = (len < 64*1024) ? len : 64*1024;
-		n = pread(pack_fd, inbuf, n, from);
+		n = pread_threadsafe(pack_fd, inbuf, n, from);
 		if (n < 0)
 			die_errno("cannot pread pack file");
 		if (!n)
-- 8< --