Git development
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: "Gary V. Vaughan" <git@mlists.thewrittenword.com>
Cc: git@vger.kernel.org, Johannes Sixt <j6t@kdbg.org>
Subject: Re: Support pthread with no recursive mutex (SunOS 5.6)
Date: Tue, 2 Nov 2010 12:35:10 -0500	[thread overview]
Message-ID: <20101102173510.GB5636@burratino> (raw)
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.

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

  reply	other threads:[~2010-11-02 17:36 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-02 14:12 Support pthread with no recursive mutex (SunOS 5.6) Gary V. Vaughan
2010-11-02 17:35 ` Jonathan Nieder [this message]
2010-11-04 15:01   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101102173510.GB5636@burratino \
    --to=jrnieder@gmail.com \
    --cc=git@mlists.thewrittenword.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox