Support pthread with no recursive mutex (SunOS 5.6)

Support pthread with no recursive mutex (SunOS 5.6)

[Gary V. Vaughan]

Thanks for merging my last patch series into the new release.  git 1.7.3.2
now compiles correctly on all of our hosts, save Solaris 2.6 (SunOS 5.6)
which has no recursive mutex support in its pthreads.  While we could
penalize that platform by disabling threads entirely, they worked perfectly
well up until this release.

This patch adds a new configure and Makefile setting to selectively disable
just the single recursive mutex usage in favour of the older implementation
that used a simple non-recursive mutex in the same spot, but on just Solaris
2.6.

Signed-off-by: Gary V. Vaughan <gary@thewrittenword.com>
---
 Makefile               |    4 ++++
 builtin/pack-objects.c |    4 ++++
 config.mak.in          |    1 +
 configure.ac           |   16 ++++++++++++++++
 thread-utils.c         |    2 ++
 thread-utils.h         |    2 ++
 6 files changed, 29 insertions(+)

Index: b/Makefile
===================================================================
--- a/Makefile
+++ b/Makefile
@@ -119,6 +119,9 @@ all::
 #
 # Define NO_PTHREADS if you do not have or do not want to use Pthreads.
 #
+# Define NO_RECURSIVE_MUTEX if you do have Pthreads, but with no support for
+# recursive mutexs (SunOS 5.6).
+#
 # Define NO_PREAD if you have a problem with pread() system call (e.g.
 # cygwin1.dll before v1.5.22).
 #
@@ -847,6 +850,7 @@ ifeq ($(uname_S),SunOS)
 		SOCKLEN_T = int
 		NO_HSTRERROR = YesPlease
 		NO_IPV6 = YesPlease
+		NO_RECURSIVE_MUTEX = YesPlease
 		NO_SOCKADDR_STORAGE = YesPlease
 		NO_UNSETENV = YesPlease
 		NO_SETENV = YesPlease
Index: b/builtin/pack-objects.c
===================================================================
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1561,7 +1561,11 @@ static pthread_cond_t progress_cond;
  */
 static void init_threaded_search(void)
 {
+#ifndef NO_RECURSIVE_MUTEX
 	init_recursive_mutex(&read_mutex);
+#else
+	pthread_mutex_init(&read_mutex, NULL);
+#endif
 	pthread_mutex_init(&cache_mutex, NULL);
 	pthread_mutex_init(&progress_mutex, NULL);
 	pthread_cond_init(&progress_cond, NULL);
Index: b/configure.ac
===================================================================
--- a/configure.ac
+++ b/configure.ac
@@ -876,6 +876,9 @@ AC_SUBST(NO_MKSTEMPS)
 # Define NO_PTHREADS if we do not have pthreads.
 #
 # Define PTHREAD_LIBS to the linker flag used for Pthread support.
+#
+# Define NO_RECURSIVE_MUTEX if you do have Pthreads, but with no support for
+# recursive mutexes (SunOS 5.6).
 AC_DEFUN([PTHREADTEST_SRC], [
 #include <pthread.h>
 
@@ -940,9 +943,22 @@ fi
 
 CFLAGS="$old_CFLAGS"
 
+if test $threads_found = yes; then
+  AC_MSG_CHECKING([Checking for PTHREAD_MUTEX_RECURSIVE support])
+  AC_EGREP_HEADER([PTHREAD_MUTEX_RECURSIVE], [pthread.h],
+	[], [NO_RECURSIVE_MUTEX=YesPlease])
+  if test -n "$NO_RECURSIVE_MUTEX"; then
+    AC_MSG_RESULT([no])
+  else
+    AC_MSG_RESULT([yes])
+  fi
+fi
+
+
 AC_SUBST(PTHREAD_CFLAGS)
 AC_SUBST(PTHREAD_LIBS)
 AC_SUBST(NO_PTHREADS)
+AC_SUBST(NO_RECURSIVE_MUTEX)
 
 ## Output files
 AC_CONFIG_FILES(["${config_file}":"${config_in}":"${config_append}"])
Index: b/thread-utils.c
===================================================================
--- a/thread-utils.c
+++ b/thread-utils.c
@@ -45,6 +45,7 @@ int online_cpus(void)
 	return 1;
 }
 
+#ifndef NO_RECURSIVE_MUTEX
 int init_recursive_mutex(pthread_mutex_t *m)
 {
 	pthread_mutexattr_t a;
@@ -59,3 +60,4 @@ int init_recursive_mutex(pthread_mutex_t
 	}
 	return ret;
 }
+#endif
Index: b/thread-utils.h
===================================================================
--- a/thread-utils.h
+++ b/thread-utils.h
@@ -2,6 +2,8 @@
 #define THREAD_COMPAT_H
 
 extern int online_cpus(void);
+#ifndef NO_RECURSIVE_MUTEX
 extern int init_recursive_mutex(pthread_mutex_t*);
+#endif
 
 #endif /* THREAD_COMPAT_H */
Index: b/config.mak.in
===================================================================
--- a/config.mak.in
+++ b/config.mak.in
@@ -66,5 +66,6 @@ SOCKLEN_T=@SOCKLEN_T@
 FREAD_READS_DIRECTORIES=@FREAD_READS_DIRECTORIES@
 SNPRINTF_RETURNS_BOGUS=@SNPRINTF_RETURNS_BOGUS@
 NO_PTHREADS=@NO_PTHREADS@
+NO_RECURSIVE_MUTEX=@NO_RECURSIVE_MUTEX@
 PTHREAD_CFLAGS=@PTHREAD_CFLAGS@
 PTHREAD_LIBS=@PTHREAD_LIBS@
--
Gary V. Vaughan (gary@thewrittenword.com)

Re: Support pthread with no recursive mutex (SunOS 5.6)

[Jonathan Nieder]

Hi Gary,

Gary V. Vaughan wrote:

> Thanks for merging my last patch series into the new release.  git 1.7.3.2
> now compiles correctly on all of our hosts, save Solaris 2.6 (SunOS 5.6)
> which has no recursive mutex support in its pthreads.

Nice.

> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -1561,7 +1561,11 @@ static pthread_cond_t progress_cond;
>   */
>  static void init_threaded_search(void)
>  {
> +#ifndef NO_RECURSIVE_MUTEX
>  	init_recursive_mutex(&read_mutex);
> +#else
> +	pthread_mutex_init(&read_mutex, NULL);
> +#endif

Wouldn't that defeat the purpose of using a recursive mutex in the first
place?  Let's see...

$ git log -m --first-parent -S'init_recursive_mutex' -- builtin/pack-objects.c
commit ea5f75a64ae52590b06713d45d84de03ca109ccc
Merge: af65543 9374919
Author: Junio C Hamano <gitster@pobox.com>
Date:   Fri May 21 04:02:16 2010 -0700

    Merge branch 'np/malloc-threading'
    
    * np/malloc-threading:
      Thread-safe xmalloc and xrealloc needs a recursive mutex
      Make xmalloc and xrealloc thread-safe
$ git log -S'init_recursive_mutex' af65543..9374919
commit 937491944292fa3303b565b9bd8914c6b644ab13
Author: Johannes Sixt <j6t@kdbg.org>
Date:   Thu Apr 8 09:15:39 2010 +0200

    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 <frekui@gmail.com>
    Signed-off-by: Johannes Sixt <j6t@kdbg.org>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

Presumably the problem scenarios are something like this:

	ll_find_deltas():
	  init_threaded_search() [sets try_to_free_routine]
	  threaded_find_deltas() ->
	   find_deltas() ->
	    try_delta() [acquires read_lock] ->
	     read_sha1_file() ->
	      read_object() ->
	       xmemdupz() ->
	        xmallocz() ->
	         xmalloc() ->
	          try_to_free_from_threads() ->
	           read_lock() --- deadlock.

Hope that helps,
Jonathan

Re: Support pthread with no recursive mutex (SunOS 5.6)

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Hi Gary,
>
> Gary V. Vaughan wrote:
>
>> Thanks for merging my last patch series into the new release.  git 1.7.3.2
>> now compiles correctly on all of our hosts, save Solaris 2.6 (SunOS 5.6)
>> which has no recursive mutex support in its pthreads.
>
> Nice.
>
>> --- a/builtin/pack-objects.c
>> +++ b/builtin/pack-objects.c
>> @@ -1561,7 +1561,11 @@ static pthread_cond_t progress_cond;
>>   */
>>  static void init_threaded_search(void)
>>  {
>> +#ifndef NO_RECURSIVE_MUTEX
>>  	init_recursive_mutex(&read_mutex);
>> +#else
>> +	pthread_mutex_init(&read_mutex, NULL);
>> +#endif
>
> Wouldn't that defeat the purpose of using a recursive mutex in the first
> place?

Thanks for a sanity.

What might make sense is not NO_RECURSIVE_MUTEX but MUTEX_IS_RECURSIVE
(which would be the Windows case).  If Solaris 2.6 has mutex without
PTHREAD_MUTEX_RECURSIVE, and its mutex is already recursive without being
told anything special, then something like the above patch (#ifdef should
be in the definition of init_recursive_mutex() function, not its callsite,
by the way) would be a good thing to have, but I somehow doubt that it
would be the case...