From a9a746364bd26d333c7229c6f7e851b507cd284a Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Wed, 24 Mar 2010 16:22:34 -0400 Subject: Make xmalloc and xrealloc thread-safe By providing a hook for the routine responsible for trying to free some memory on malloc failure, we can ensure that the called routine is protected by the appropriate locks when threads are in play. The obvious offender here was pack-objects which was calling xmalloc() within threads while release_pack_memory() is not thread safe. Signed-off-by: Nicolas Pitre Signed-off-by: Junio C Hamano diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 539e75d..0ecc198 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -1549,6 +1549,13 @@ static void find_deltas(struct object_entry **list, unsigned *list_size, #ifndef NO_PTHREADS +static void try_to_free_from_threads(size_t size) +{ + read_lock(); + release_pack_memory(size, -1); + read_unlock(); +} + /* * The main thread waits on the condition that (at least) one of the workers * has stopped working (which is indicated in the .working member of @@ -1583,10 +1590,12 @@ static void init_threaded_search(void) pthread_mutex_init(&cache_mutex, NULL); pthread_mutex_init(&progress_mutex, NULL); pthread_cond_init(&progress_cond, NULL); + set_try_to_free_routine(try_to_free_from_threads); } static void cleanup_threaded_search(void) { + set_try_to_free_routine(NULL); pthread_cond_destroy(&progress_cond); pthread_mutex_destroy(&read_mutex); pthread_mutex_destroy(&cache_mutex); diff --git a/git-compat-util.h b/git-compat-util.h index a3c4537..1c171db 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -346,6 +346,8 @@ static inline char *gitstrchrnul(const char *s, int c) extern void release_pack_memory(size_t, int); +extern void set_try_to_free_routine(void (*routine)(size_t)); + extern char *xstrdup(const char *str); extern void *xmalloc(size_t size); extern void *xmallocz(size_t size); diff --git a/wrapper.c b/wrapper.c index 9c71b21..62edb57 100644 --- a/wrapper.c +++ b/wrapper.c @@ -3,11 +3,23 @@ */ #include "cache.h" +static void try_to_free_builtin(size_t size) +{ + release_pack_memory(size, -1); +} + +static void (*try_to_free_routine)(size_t size) = try_to_free_builtin; + +void set_try_to_free_routine(void (*routine)(size_t)) +{ + try_to_free_routine = (routine) ? routine : try_to_free_builtin; +} + char *xstrdup(const char *str) { char *ret = strdup(str); if (!ret) { - release_pack_memory(strlen(str) + 1, -1); + try_to_free_routine(strlen(str) + 1); ret = strdup(str); if (!ret) die("Out of memory, strdup failed"); @@ -21,7 +33,7 @@ void *xmalloc(size_t size) if (!ret && !size) ret = malloc(1); if (!ret) { - release_pack_memory(size, -1); + try_to_free_routine(size); ret = malloc(size); if (!ret && !size) ret = malloc(1); @@ -67,7 +79,7 @@ void *xrealloc(void *ptr, size_t size) if (!ret && !size) ret = realloc(ptr, 1); if (!ret) { - release_pack_memory(size, -1); + try_to_free_routine(size); ret = realloc(ptr, size); if (!ret && !size) ret = realloc(ptr, 1); @@ -83,7 +95,7 @@ void *xcalloc(size_t nmemb, size_t size) if (!ret && (!nmemb || !size)) ret = calloc(1, 1); if (!ret) { - release_pack_memory(nmemb * size, -1); + try_to_free_routine(nmemb * size); ret = calloc(nmemb, size); if (!ret && (!nmemb || !size)) ret = calloc(1, 1); -- cgit v0.10.2-6-g49f6 From 937491944292fa3303b565b9bd8914c6b644ab13 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Thu, 8 Apr 2010 09:15:39 +0200 Subject: Thread-safe xmalloc and xrealloc needs a recursive mutex The mutex used to protect object access (read_mutex) may need to be acquired recursively. Introduce init_recursive_mutex() helper function in thread-utils.c that constructs a mutex with the PHREAD_MUTEX_RECURSIVE attribute. pthread_mutex_init() emulation on Win32 is already recursive as it is implemented on top of the CRITICAL_SECTION type, which is recursive. http://msdn.microsoft.com/en-us/library/ms682530%28VS.85%29.aspx Add do-nothing compatibility wrappers for pthread_mutexattr* functions. Initial-version-by: Fredrik Kuivinen Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano diff --git a/builtin-grep.c b/builtin-grep.c index 371db0a..52137f4 100644 --- a/builtin-grep.c +++ b/builtin-grep.c @@ -16,8 +16,8 @@ #include "quote.h" #ifndef NO_PTHREADS -#include "thread-utils.h" #include +#include "thread-utils.h" #endif static char const * const grep_usage[] = { diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 0ecc198..26fc7cd 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -18,8 +18,8 @@ #include "refs.h" #ifndef NO_PTHREADS -#include "thread-utils.h" #include +#include "thread-utils.h" #endif static const char pack_usage[] = @@ -1586,7 +1586,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/compat/win32/pthread.h b/compat/win32/pthread.h index c72f100..a45f8d6 100644 --- a/compat/win32/pthread.h +++ b/compat/win32/pthread.h @@ -18,11 +18,17 @@ */ #define pthread_mutex_t CRITICAL_SECTION -#define pthread_mutex_init(a,b) InitializeCriticalSection((a)) +#define pthread_mutex_init(a,b) (InitializeCriticalSection((a)), 0) #define pthread_mutex_destroy(a) DeleteCriticalSection((a)) #define pthread_mutex_lock EnterCriticalSection #define pthread_mutex_unlock LeaveCriticalSection +typedef int pthread_mutexattr_t; +#define pthread_mutexattr_init(a) (*(a) = 0) +#define pthread_mutexattr_destroy(a) do {} while (0) +#define pthread_mutexattr_settype(a, t) 0 +#define PTHREAD_MUTEX_RECURSIVE 0 + /* * Implement simple condition variable for Windows threads, based on ACE * implementation. diff --git a/thread-utils.c b/thread-utils.c index 4f9c829..589f838 100644 --- a/thread-utils.c +++ b/thread-utils.c @@ -1,4 +1,5 @@ #include "cache.h" +#include #if defined(hpux) || defined(__hpux) || defined(_hpux) # include @@ -43,3 +44,18 @@ int online_cpus(void) return 1; } + +int init_recursive_mutex(pthread_mutex_t *m) +{ + pthread_mutexattr_t a; + int ret; + + ret = pthread_mutexattr_init(&a); + if (!ret) { + ret = pthread_mutexattr_settype(&a, PTHREAD_MUTEX_RECURSIVE); + if (!ret) + ret = pthread_mutex_init(m, &a); + pthread_mutexattr_destroy(&a); + } + return ret; +} 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 */ -- cgit v0.10.2-6-g49f6