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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Nov 2, 2010, 17:35 UTC
Message-ID: <20101102173510.GB5636@burratino>
In-Reply-To: <20101102141227.GA3991@thor.il.thewrittenword.com>

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.

Show 11 quoted lines

> --- 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