From 851c34b04e0ce866e15c28e144986eca7533a6f4 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Sat, 8 May 2010 17:13:49 +0200 Subject: [PATCH 1/2] Have set_try_to_free_routine return the previous routine This effectively requires from the callers of set_try_to_free_routine to treat the try-to-free-routines as a stack. We will need this for the next patch where the only current caller cannot depend on that the previously set routine was the default routine. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- builtin-pack-objects.c | 6 ++++-- git-compat-util.h | 3 ++- wrapper.c | 6 ++++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 26fc7cd5a6..5279cd99f2 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -1556,6 +1556,8 @@ static void try_to_free_from_threads(size_t size) read_unlock(); } +try_to_free_t old_try_to_free_routine; + /* * 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 @@ -1590,12 +1592,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); + old_try_to_free_routine = set_try_to_free_routine(try_to_free_from_threads); } static void cleanup_threaded_search(void) { - set_try_to_free_routine(NULL); + set_try_to_free_routine(old_try_to_free_routine); 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 1c171db8b1..828aadaf3a 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -346,7 +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)); +typedef void (*try_to_free_t)(size_t); +extern try_to_free_t set_try_to_free_routine(try_to_free_t); extern char *xstrdup(const char *str); extern void *xmalloc(size_t size); diff --git a/wrapper.c b/wrapper.c index 62edb57338..8aa9df9b83 100644 --- a/wrapper.c +++ b/wrapper.c @@ -10,9 +10,11 @@ static void try_to_free_builtin(size_t size) 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_t set_try_to_free_routine(try_to_free_t routine) { - try_to_free_routine = (routine) ? routine : try_to_free_builtin; + try_to_free_t old = try_to_free_routine; + try_to_free_routine = routine; + return old; } char *xstrdup(const char *str) From 3a0942598ce33b195bfaaf250b2da23e4eceb3c6 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Sat, 8 May 2010 17:18:06 +0200 Subject: [PATCH 2/2] Do not call release_pack_memory in malloc wrappers when GIT_TRACE is used This avoids a potential race condition when async procedures are implemented as threads where release_pack_memory() can be called from different threads without locking under memory pressure. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- trace.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/trace.c b/trace.c index 4229ae1231..1e560cb0b9 100644 --- a/trace.c +++ b/trace.c @@ -25,6 +25,10 @@ #include "cache.h" #include "quote.h" +void do_nothing(size_t unused) +{ +} + /* Get a trace file descriptor from GIT_TRACE env variable. */ static int get_trace_fd(int *need_close) { @@ -72,6 +76,7 @@ void trace_printf(const char *fmt, ...) if (!fd) return; + set_try_to_free_routine(do_nothing); /* is never reset */ strbuf_init(&buf, 64); va_start(ap, fmt); len = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap); @@ -103,6 +108,7 @@ void trace_argv_printf(const char **argv, const char *fmt, ...) if (!fd) return; + set_try_to_free_routine(do_nothing); /* is never reset */ strbuf_init(&buf, 64); va_start(ap, fmt); len = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap);