Re: [PATCH v2] Make xmalloc and xrealloc thread-safe

[Fredrik Kuivinen <frekui@gmail.com>]

On Wed, Apr 07, 2010 at 09:17:04AM -0400, Nicolas Pitre wrote:
> On Wed, 7 Apr 2010, Shawn Pearce wrote:
> > You mentioned avoiding a recursive mutex only because windows
> > emulation doesn't have support for it.  But that's exactly what we
> > need here.  Shouldn't windows have a recursive mutex object that can
> > just be used inside of the emulation layer when we really need a
> > recursive mutex?
> 
> Maybe.  That would in fact just mean pushing the double mutex issue into 
> the pthread emulation instead of having it outside it.  This would 
> impact performances for all mutexes although only one instance of them 
> currently require a recursive behavior.

As I mentioned in another mail in this thread, our mutex
implementation on WIN32 already is recursive. It is implemented on top
of the CRITICAL_SECTION type, which is recursive. See
http://msdn.microsoft.com/en-us/library/ms682530%28VS.85%29.aspx

We only need something like the following (on top of Nico's previous
patch). Warning: It hasn't even been compile tested on WIN32.

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 65f797f..19e42cf 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1559,7 +1559,7 @@ static pthread_cond_t progress_cond;
  */
 static void init_threaded_search(void)
 {
-	pthread_mutex_init(&read_mutex, NULL);
+	init_recursive_mutex(&read_mutex);
 	pthread_mutex_init(&cache_mutex, NULL);
 	pthread_mutex_init(&progress_mutex, NULL);
 	pthread_cond_init(&progress_cond, NULL);
diff --git a/thread-utils.c b/thread-utils.c
index 4f9c829..3c8d817 100644
--- a/thread-utils.c
+++ b/thread-utils.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include <pthread.h>
 
 #if defined(hpux) || defined(__hpux) || defined(_hpux)
 #  include <sys/pstat.h>
@@ -43,3 +44,24 @@ int online_cpus(void)
 
 	return 1;
 }
+
+int init_recursive_mutex(pthread_mutex_t *m)
+{
+#ifdef _WIN32
+	/* The mutexes in the WIN32 pthreads emulation layer are
+	 * recursive, so we don't have to do anything extra here. */
+	return pthread_mutex_init(m, NULL);
+#else
+	pthread_mutexattr_t a;
+	int ret;
+	if (pthread_mutexattr_init(&a))
+		die("pthread_mutexattr_init failed: %s", strerror(errno));
+
+	if (pthread_mutexattr_settype(&a, PTHREAD_MUTEX_RECURSIVE))
+		die("pthread_mutexattr_settype failed: %s", strerror(errno));
+
+	ret = pthread_mutex_init(m, &a);
+	pthread_mutexattr_destroy(&a);
+	return ret;
+#endif
+}
diff --git a/thread-utils.h b/thread-utils.h
index cce4b77..1727a03 100644
--- a/thread-utils.h
+++ b/thread-utils.h
@@ -2,5 +2,6 @@
 #define THREAD_COMPAT_H
 
 extern int online_cpus(void);
+extern int init_recursive_mutex(pthread_mutex_t*);
 
 #endif /* THREAD_COMPAT_H */